PR middle-end/44813 (ICE in Mozilla build in ptr_deref_may_alias_decl_p)
Richard Guenther
rguenther@suse.de
Tue Jul 6 09:20:00 GMT 2010
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<nsTextFrame*>(this)->EnsureTextRun();
> + PropertyProvider provider(const_cast<nsTextFrame*>(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 <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex
More information about the Gcc-patches
mailing list