From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26794 invoked by alias); 6 Jul 2010 09:20:38 -0000 Received: (qmail 26775 invoked by uid 22791); 6 Jul 2010 09:20:32 -0000 X-SWARE-Spam-Status: No, hits=-3.3 required=5.0 tests=AWL,BAYES_00,TW_GF,TW_TM,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 06 Jul 2010 09:20:17 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) by mx2.suse.de (Postfix) with ESMTP id D722986A2E; Tue, 6 Jul 2010 11:20:13 +0200 (CEST) Date: Tue, 06 Jul 2010 09:20:00 -0000 From: Richard Guenther To: Jan Hubicka Cc: gcc-patches@gcc.gnu.org Subject: Re: PR middle-end/44813 (ICE in Mozilla build in ptr_deref_may_alias_decl_p) In-Reply-To: <20100705161839.GA14621@atrey.karlin.mff.cuni.cz> Message-ID: References: <20100704224222.GB29130@kam.mff.cuni.cz> <20100705102818.GD29130@kam.mff.cuni.cz> <20100705161839.GA14621@atrey.karlin.mff.cuni.cz> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2010-07/txt/msg00435.txt.bz2 On Mon, 5 Jul 2010, Jan Hubicka wrote: > Hi, > here is updated patch, with Richard's reduced testcase that solves some furhter > problems. > In particular adding the tester down that ipa-split produce new SSA name > for return value and sets as definining statement the call (that is wrong > for DECL_BY_REFERENCE)> I also cleaned up the code a bit there. > Also ipa-ssa-ccp considers RESULT_DECL as undefined that leads to misoptimization > after fixing the first problem on libstdc++ vector testcase. > > Bootstrapped/regtested x86_64-linux, OK? > > * tree-ssa-uninit.c (ssa_undefined_value_p): Result decl is defined > for functions passed by reference. > * tree.c (needs_to_live_in_memory): RESULT_DECL don't need to live > in memory when passed by reference. > * tree-ssa-ccp.c (get_default_value): Only VAR_DECL is undefined at > beggining. > * ipa-split.c (split_function): Cleanup way return value is passed; > handle SSA DECL_BY_REFERENCE retvals. > * tree-ssa.c (verify_def): Verify that RESULT_DECL is read only when > DECL_BY_REFERENCE is set. > * tree-inline.c (remap_gimple_stmt): Handle SSA DECL_BY_REFERENCE > returns. > * tree-ssa-structalias.c (get_constraint_for_ssa_var, get_fi_for_callee, > find_what_p_points_to): Handle RESULT_DECL. > > * testsuite/g++.dg/torture/pr44813.C: New testcase. > Index: tree-ssa-uninit.c > =================================================================== > *** tree-ssa-uninit.c (revision 161826) > --- tree-ssa-uninit.c (working copy) > *************** ssa_undefined_value_p (tree t) > *** 92,97 **** > --- 92,103 ---- > if (TREE_CODE (var) == PARM_DECL) > return false; > > + /* When returning by reference the return address is actually a hidden > + parameter. */ > + if (TREE_CODE (SSA_NAME_VAR (t)) == RESULT_DECL > + && DECL_BY_REFERENCE (SSA_NAME_VAR (t))) > + return false; > + > /* Hard register variables get their initial value from the ether. */ > if (TREE_CODE (var) == VAR_DECL && DECL_HARD_REGISTER (var)) > return false; > Index: tree.c > =================================================================== > *** tree.c (revision 161826) > --- tree.c (working copy) > *************** needs_to_live_in_memory (const_tree t) > *** 9750,9755 **** > --- 9750,9756 ---- > return (TREE_ADDRESSABLE (t) > || is_global_var (t) > || (TREE_CODE (t) == RESULT_DECL > + && !DECL_BY_REFERENCE (t) > && aggregate_value_p (t, current_function_decl))); > } > > Index: testsuite/g++.dg/torture/pr44813.C > =================================================================== > *** testsuite/g++.dg/torture/pr44813.C (revision 0) > --- testsuite/g++.dg/torture/pr44813.C (revision 0) > *************** > *** 0 **** > --- 1,60 ---- > + typedef unsigned int PRUint32; > + typedef int PRInt32; > + typedef unsigned long PRUint64; > + typedef int PRIntn; > + typedef PRIntn PRBool; > + struct nsRect { > + nsRect(const nsRect& aRect) { } > + }; > + enum nsCompatibility { eCompatibility_NavQuirks = 3 }; > + class gfxContext; > + typedef PRUint64 nsFrameState; > + class nsPresContext { > + public: > + nsCompatibility CompatibilityMode() const { } > + }; > + class nsStyleContext { > + public: > + PRBool HasTextDecorations() const; > + }; > + class nsIFrame { > + public: > + nsPresContext* PresContext() const; > + nsStyleContext* GetStyleContext() const; > + nsFrameState GetStateBits() const; > + nsRect GetOverflowRect() const; > + }; > + class nsFrame : public nsIFrame { }; > + class nsLineList_iterator { }; > + class nsLineList { > + public: > + typedef nsLineList_iterator iterator; > + }; > + class gfxSkipCharsIterator { }; > + class gfxTextRun { > + public: > + class PropertyProvider { }; > + }; > + class nsTextFrame : public nsFrame > + { > + virtual nsRect ComputeTightBounds(gfxContext* aContext) const; > + gfxSkipCharsIterator EnsureTextRun(gfxContext* aReferenceContext = 0L, > + nsIFrame* aLineContainer = 0L, > + const nsLineList::iterator* aLine = 0L, > + PRUint32* aFlowEndInTextRun = 0L); > + }; > + class PropertyProvider : public gfxTextRun::PropertyProvider > + { > + public: > + PropertyProvider(nsTextFrame* aFrame, const gfxSkipCharsIterator& aStart); > + PRInt32 mLength[64]; > + }; > + nsRect nsTextFrame::ComputeTightBounds(gfxContext* aContext) const > + { > + if ((GetStyleContext()->HasTextDecorations() > + && eCompatibility_NavQuirks == PresContext()->CompatibilityMode()) > + || (GetStateBits() & (nsFrameState(1) << (23)))) > + return GetOverflowRect(); > + gfxSkipCharsIterator iter = const_cast(this)->EnsureTextRun(); > + PropertyProvider provider(const_cast(this), iter); > + } > Index: tree-ssa-ccp.c > =================================================================== > *** tree-ssa-ccp.c (revision 161826) > --- tree-ssa-ccp.c (working copy) > *************** get_default_value (tree var) > *** 300,306 **** > before being initialized. If VAR is a local variable, we > can assume initially that it is UNDEFINED, otherwise we must > consider it VARYING. */ > ! if (is_gimple_reg (sym) && TREE_CODE (sym) != PARM_DECL) > val.lattice_val = UNDEFINED; > else > val.lattice_val = VARYING; > --- 300,307 ---- > before being initialized. If VAR is a local variable, we > can assume initially that it is UNDEFINED, otherwise we must > consider it VARYING. */ > ! if (is_gimple_reg (sym) > ! && TREE_CODE (sym) == VAR_DECL) > val.lattice_val = UNDEFINED; > else > val.lattice_val = VARYING; > Index: ipa-split.c > =================================================================== > *** ipa-split.c (revision 161826) > --- ipa-split.c (working copy) > *************** split_function (struct split_point *spli > *** 967,997 **** > return_bb == EXIT_BLOCK_PTR ? 0 : EDGE_FALLTHRU); > e->count = call_bb->count; > e->probability = REG_BR_PROB_BASE; > if (return_bb != EXIT_BLOCK_PTR) > { > real_retval = retval = find_retval (return_bb); > if (real_retval > && !is_gimple_min_invariant (retval) > && (TREE_CODE (retval) != SSA_NAME > ! || !SSA_NAME_IS_DEFAULT_DEF (retval))) > { > gimple_stmt_iterator psi; > > ! /* See if there is PHI defining return value. */ > ! for (psi = gsi_start_phis (return_bb); > ! !gsi_end_p (psi); gsi_next (&psi)) > ! if (is_gimple_reg (gimple_phi_result (gsi_stmt (psi)))) > ! break; > ! > ! /* When we have PHI, update PHI. When there is no PHI, > ! update the return statement itself. */ > ! if (TREE_CODE (retval) == SSA_NAME) > { > retval = make_ssa_name (SSA_NAME_VAR (retval), call); > if (TREE_CODE (retval) == SSA_NAME > && !gsi_end_p (psi)) > add_phi_arg (gsi_stmt (psi), retval, e, UNKNOWN_LOCATION); > ! else if (TREE_CODE (retval) == SSA_NAME) > { > gimple_stmt_iterator bsi; > for (bsi = gsi_start_bb (return_bb); !gsi_end_p (bsi); > --- 967,1013 ---- > return_bb == EXIT_BLOCK_PTR ? 0 : EDGE_FALLTHRU); > e->count = call_bb->count; > e->probability = REG_BR_PROB_BASE; > + > + /* If there is return basic block, see what value we need to store > + return value into and put call just before it. */ > if (return_bb != EXIT_BLOCK_PTR) > { > real_retval = retval = find_retval (return_bb); > + > + /* See if return value is computed by split part; > + function might just return its argument, invariant or undefined > + value. In this case we don't need to do any updating. */ > if (real_retval > && !is_gimple_min_invariant (retval) > && (TREE_CODE (retval) != SSA_NAME > ! || (!SSA_NAME_IS_DEFAULT_DEF (retval) > ! || DECL_BY_REFERENCE > ! (DECL_RESULT (current_function_decl))))) > { > gimple_stmt_iterator psi; > > ! /* See if we need new SSA_NAME for the result. > ! When DECL_BY_REFERENCE is true, retval is actually pointer to > ! return value and it is constant in whole function. */ > ! if (TREE_CODE (retval) == SSA_NAME > ! && !DECL_BY_REFERENCE (DECL_RESULT (current_function_decl))) > { > retval = make_ssa_name (SSA_NAME_VAR (retval), call); > + > + /* See if there is PHI defining return value. */ > + for (psi = gsi_start_phis (return_bb); > + !gsi_end_p (psi); gsi_next (&psi)) > + if (is_gimple_reg (gimple_phi_result (gsi_stmt (psi)))) > + break; > + > + /* When there is PHI, just update its value. */ > if (TREE_CODE (retval) == SSA_NAME > && !gsi_end_p (psi)) > add_phi_arg (gsi_stmt (psi), retval, e, UNKNOWN_LOCATION); > ! /* Otherwise update the return BB itself. > ! find_return_bb allows at most one assignment to return value, > ! so update first statement. */ > ! else > { > gimple_stmt_iterator bsi; > for (bsi = gsi_start_bb (return_bb); !gsi_end_p (bsi); > *************** split_function (struct split_point *spli > *** 1016,1021 **** > --- 1032,1040 ---- > } > gsi_insert_after (&gsi, call, GSI_NEW_STMT); > } > + /* We don't use return block (there is either no return in function or > + multiple of them). So create new basic block with return statement. > + */ > else > { > gimple ret; > *************** split_function (struct split_point *spli > *** 1030,1036 **** > && !DECL_BY_REFERENCE (retval)) > retval = create_tmp_reg (TREE_TYPE (retval), NULL); > if (is_gimple_reg (retval)) > ! retval = make_ssa_name (retval, call); > if (DECL_BY_REFERENCE (DECL_RESULT (current_function_decl))) > gimple_call_set_lhs (call, build_simple_mem_ref (retval)); > else > --- 1049,1076 ---- > && !DECL_BY_REFERENCE (retval)) > retval = create_tmp_reg (TREE_TYPE (retval), NULL); > if (is_gimple_reg (retval)) > ! { > ! /* When returning by reference, there is only one SSA name > ! assigned to RESULT_DECL (that is pointer to return value). > ! Look it up or create new one if it is missing. */ > ! if (DECL_BY_REFERENCE (retval)) > ! { > ! tree retval_name; > ! if ((retval_name = gimple_default_def (cfun, retval)) > ! != NULL) > ! retval = retval_name; > ! else > ! { > ! retval_name = make_ssa_name (retval, > ! gimple_build_nop ()); > ! set_default_def (retval, retval_name); > ! retval = retval_name; > ! } > ! } > ! /* Otherwise produce new SSA name for return value. */ > ! else > ! retval = make_ssa_name (retval, call); > ! } > if (DECL_BY_REFERENCE (DECL_RESULT (current_function_decl))) > gimple_call_set_lhs (call, build_simple_mem_ref (retval)); > else > Index: tree-ssa.c > =================================================================== > *** tree-ssa.c (revision 161826) > --- tree-ssa.c (working copy) > *************** verify_def (basic_block bb, basic_block > *** 638,643 **** > --- 638,650 ---- > if (verify_ssa_name (ssa_name, is_virtual)) > goto err; > > + if (TREE_CODE (SSA_NAME_VAR (ssa_name)) == RESULT_DECL > + && DECL_BY_REFERENCE (SSA_NAME_VAR (ssa_name))) > + { > + error ("RESULT_DECL should be read only when DECL_BY_REFERENCE is set."); > + goto err; > + } > + > if (definition_block[SSA_NAME_VERSION (ssa_name)]) > { > error ("SSA_NAME created in two different blocks %i and %i", > Index: tree-inline.c > =================================================================== > *** tree-inline.c (revision 161826) > --- tree-inline.c (working copy) > *************** remap_gimple_stmt (gimple stmt, copy_bod > *** 1236,1242 **** > If RETVAL is just the result decl, the result decl has > already been set (e.g. a recent "foo (&result_decl, ...)"); > just toss the entire GIMPLE_RETURN. */ > ! if (retval && TREE_CODE (retval) != RESULT_DECL) > { > copy = gimple_build_assign (id->retvar, retval); > /* id->retvar is already substituted. Skip it on later remapping. */ > --- 1237,1246 ---- > If RETVAL is just the result decl, the result decl has > already been set (e.g. a recent "foo (&result_decl, ...)"); > just toss the entire GIMPLE_RETURN. */ > ! if (retval > ! && (TREE_CODE (retval) != RESULT_DECL > ! && (TREE_CODE (retval) != SSA_NAME > ! || TREE_CODE (SSA_NAME_VAR (retval)) != RESULT_DECL))) > { > copy = gimple_build_assign (id->retvar, retval); > /* id->retvar is already substituted. Skip it on later remapping. */ > Index: tree-ssa-structalias.c > =================================================================== > *** tree-ssa-structalias.c (revision 161826) > --- tree-ssa-structalias.c (working copy) > *************** get_constraint_for_ssa_var (tree t, VEC( > *** 2836,2842 **** > /* For parameters, get at the points-to set for the actual parm > decl. */ > if (TREE_CODE (t) == SSA_NAME > ! && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL > && SSA_NAME_IS_DEFAULT_DEF (t)) > { > get_constraint_for_ssa_var (SSA_NAME_VAR (t), results, address_p); > --- 2836,2844 ---- > /* For parameters, get at the points-to set for the actual parm > decl. */ > if (TREE_CODE (t) == SSA_NAME > ! && (TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL > ! || (TREE_CODE (SSA_NAME_VAR (t)) == RESULT_DECL > ! && DECL_BY_REFERENCE (SSA_NAME_VAR (t)))) There shouldn't be a default-def for non-by-reference result decls, so no need to check the flag here or below. Ok with that change. Thanks, Richard. > && SSA_NAME_IS_DEFAULT_DEF (t)) > { > get_constraint_for_ssa_var (SSA_NAME_VAR (t), results, address_p); > *************** get_fi_for_callee (gimple call) > *** 3982,3988 **** > if (TREE_CODE (decl) == SSA_NAME) > { > if (TREE_CODE (decl) == SSA_NAME > ! && TREE_CODE (SSA_NAME_VAR (decl)) == PARM_DECL > && SSA_NAME_IS_DEFAULT_DEF (decl)) > decl = SSA_NAME_VAR (decl); > return get_vi_for_tree (decl); > --- 3984,3992 ---- > if (TREE_CODE (decl) == SSA_NAME) > { > if (TREE_CODE (decl) == SSA_NAME > ! && (TREE_CODE (SSA_NAME_VAR (decl)) == PARM_DECL > ! || (TREE_CODE (SSA_NAME_VAR (decl)) == RESULT_DECL > ! && DECL_BY_REFERENCE (SSA_NAME_VAR (decl)))) > && SSA_NAME_IS_DEFAULT_DEF (decl)) > decl = SSA_NAME_VAR (decl); > return get_vi_for_tree (decl); > *************** find_what_p_points_to (tree p) > *** 5751,5757 **** > /* For parameters, get at the points-to set for the actual parm > decl. */ > if (TREE_CODE (p) == SSA_NAME > ! && TREE_CODE (SSA_NAME_VAR (p)) == PARM_DECL > && SSA_NAME_IS_DEFAULT_DEF (p)) > lookup_p = SSA_NAME_VAR (p); > > --- 5755,5763 ---- > /* For parameters, get at the points-to set for the actual parm > decl. */ > if (TREE_CODE (p) == SSA_NAME > ! && (TREE_CODE (SSA_NAME_VAR (p)) == PARM_DECL > ! || (TREE_CODE (SSA_NAME_VAR (p)) == RESULT_DECL > ! && DECL_BY_REFERENCE (SSA_NAME_VAR (p)))) > && SSA_NAME_IS_DEFAULT_DEF (p)) > lookup_p = SSA_NAME_VAR (p); > > > -- Richard Guenther Novell / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex