| Message ID | 201303011238.05564.ebotcazou@adacore.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Mar 1, 2013 at 12:38 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: > Hi, > > this is the ICE during the build of the Ada runtime on x86-64/Windows: the > compiler stops on the assertion in declare_return_variable: > > var = return_slot; > gcc_assert (TREE_CODE (var) != SSA_NAME); > > The problem stems for the fnsplit pass: it turns > > P.Sin (const long_long_float x) > { > long_long_float _3; > boolean _4; > q__double _9; > > <bb 2>: > _3 = ABS_EXPR <x_2(D)>; > _4 = _3 < 1.0e+0; > if (_4 != 0) > goto <bb 3>; > else > goto <bb 4>; > > <bb 3>: > <retval> = x_2(D); > goto <bb 5>; > > <bb 4>: > _9 = q.sin (x_2(D)); > <retval> = _9; > > <bb 5>: > return <retval>; > > } > > into > > P.Sin (const long_long_float x) > { > long_long_float _3; > boolean _4; > > <bb 2>: > _3 = ABS_EXPR <x_2(D)>; > _4 = _3 < 1.0e+0; > if (_4 != 0) > goto <bb 3>; > else > goto <bb 4>; > > <bb 3>: > <retval> = x_2(D); > goto <bb 5>; > > <bb 4>: > <retval> = p.sin.part (x_2(D)); [return slot optimization] > > <bb 5>: > return <retval>; > > } > > Note the RSO flag on the call to the split part: it's bogus since the return > type is scalar: > > <real_type 0x7ffff6da02a0 long_long_float sizes-gimplified visited XF> > > Fixed by checking that either the type of the result is not is_gimple_reg_type > or the result is passed by reference before setting the RSO flag. > > Tested on x86_64-suse-linux, OK for the mainline? But in theory this pessimizes code as aggregate_value_p returned true for this? That is, isn't the bug that we rewrite a possible return-slot-decl into SSA? (do we do that here?) Thanks, Richard. > > 2013-01-01 Eric Botcazou <ebotcazou@adacore.com> > > PR tree-optimization/56424 > * ipa-split.c (split_function): Do not set the RSO flag if result is > not by reference and its type is not a register type. > > > 2013-01-01 Eric Botcazou <ebotcazou@adacore.com> > > * gcc.dg/pr56424.c: New test. > > > -- > Eric Botcazou
> But in theory this pessimizes code as aggregate_value_p returned true > for this? That is, isn't the bug that we rewrite a possible > return-slot-decl into SSA? (do we do that here?) Yes, aggregate_value_p returns true for this because the ABI says that this type is returned in memory. And, no, I don't think that this will further pessimize, given that it's already "pessimized" in gimplify_modify_expr_rhs: case CALL_EXPR: /* For calls that return in memory, give *to_p as the CALL_EXPR's return slot so that we don't generate a temporary. */ if (!CALL_EXPR_RETURN_SLOT_OPT (*from_p) && aggregate_value_p (*from_p, *from_p)) { bool use_target; if (!(rhs_predicate_for (*to_p))(*from_p)) /* If we need a temporary, *to_p isn't accurate. */ use_target = false; /* It's OK to use the return slot directly unless it's an NRV. */ else if (TREE_CODE (*to_p) == RESULT_DECL && DECL_NAME (*to_p) == NULL_TREE && needs_to_live_in_memory (*to_p)) use_target = true; else if (is_gimple_reg_type (TREE_TYPE (*to_p)) || (DECL_P (*to_p) && DECL_REGISTER (*to_p))) /* Don't force regs into memory. */ use_target = false;
On Fri, Mar 1, 2013 at 3:56 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> But in theory this pessimizes code as aggregate_value_p returned true >> for this? That is, isn't the bug that we rewrite a possible >> return-slot-decl into SSA? (do we do that here?) > > Yes, aggregate_value_p returns true for this because the ABI says that this > type is returned in memory. And, no, I don't think that this will further > pessimize, given that it's already "pessimized" in gimplify_modify_expr_rhs: > > case CALL_EXPR: > /* For calls that return in memory, give *to_p as the CALL_EXPR's > return slot so that we don't generate a temporary. */ > if (!CALL_EXPR_RETURN_SLOT_OPT (*from_p) > && aggregate_value_p (*from_p, *from_p)) > { > bool use_target; > > if (!(rhs_predicate_for (*to_p))(*from_p)) > /* If we need a temporary, *to_p isn't accurate. */ > use_target = false; > /* It's OK to use the return slot directly unless it's an NRV. */ > else if (TREE_CODE (*to_p) == RESULT_DECL > && DECL_NAME (*to_p) == NULL_TREE > && needs_to_live_in_memory (*to_p)) > use_target = true; > else if (is_gimple_reg_type (TREE_TYPE (*to_p)) > || (DECL_P (*to_p) && DECL_REGISTER (*to_p))) > /* Don't force regs into memory. */ > use_target = false; Ah, ok. Patch is ok then. Thanks, Richard. > -- > Eric Botcazou
Index: ipa-split.c =================================================================== --- ipa-split.c (revision 196253) +++ ipa-split.c (working copy) @@ -1309,7 +1309,9 @@ split_function (struct split_point *spli so return slot optimization is always possible. Moreover this is required to make DECL_BY_REFERENCE work. */ if (aggregate_value_p (DECL_RESULT (current_function_decl), - TREE_TYPE (current_function_decl))) + TREE_TYPE (current_function_decl)) + && (!is_gimple_reg_type (TREE_TYPE (DECL_RESULT (current_function_decl))) + || DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))) gimple_call_set_return_slot_opt (call, true); /* Update return value. This is bit tricky. When we do not return,
Hi, this is the ICE during the build of the Ada runtime on x86-64/Windows: the compiler stops on the assertion in declare_return_variable: var = return_slot; gcc_assert (TREE_CODE (var) != SSA_NAME); The problem stems for the fnsplit pass: it turns P.Sin (const long_long_float x) { long_long_float _3; boolean _4; q__double _9; <bb 2>: _3 = ABS_EXPR <x_2(D)>; _4 = _3 < 1.0e+0; if (_4 != 0) goto <bb 3>; else goto <bb 4>; <bb 3>: <retval> = x_2(D); goto <bb 5>; <bb 4>: _9 = q.sin (x_2(D)); <retval> = _9; <bb 5>: return <retval>; } into P.Sin (const long_long_float x) { long_long_float _3; boolean _4; <bb 2>: _3 = ABS_EXPR <x_2(D)>; _4 = _3 < 1.0e+0; if (_4 != 0) goto <bb 3>; else goto <bb 4>; <bb 3>: <retval> = x_2(D); goto <bb 5>; <bb 4>: <retval> = p.sin.part (x_2(D)); [return slot optimization] <bb 5>: return <retval>; } Note the RSO flag on the call to the split part: it's bogus since the return type is scalar: <real_type 0x7ffff6da02a0 long_long_float sizes-gimplified visited XF> Fixed by checking that either the type of the result is not is_gimple_reg_type or the result is passed by reference before setting the RSO flag. Tested on x86_64-suse-linux, OK for the mainline? 2013-01-01 Eric Botcazou <ebotcazou@adacore.com> PR tree-optimization/56424 * ipa-split.c (split_function): Do not set the RSO flag if result is not by reference and its type is not a register type. 2013-01-01 Eric Botcazou <ebotcazou@adacore.com> * gcc.dg/pr56424.c: New test.