| Message ID | 553750B2.8010209@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 22, 2015 at 9:41 AM, Tom de Vries <Tom_deVries@mentor.com> wrote: > Hi, > > this patch fixes PR65823. > > The problem is a verify_gimple ICE during compilation of > gcc.c-torture/execute/stdarg-2.c for arm at -O0/-O1: > ... > In function 'f3': > src/gcc/testsuite/gcc.c-torture/execute/stdarg-2.c:61:1: > error: incorrect sharing of tree nodes > aps[4] > # .MEM_5 = VDEF <.MEM_11> > aps[4] = aps[4]; > ... > > > Before gimplification, f3 looks like this in the original dump: > ... > struct va_list aps[10]; > > struct va_list aps[10]; > __builtin_va_start ((struct &) (struct *) &aps[4], i); > x = VA_ARG_EXPR <aps[4]>; > __builtin_va_end ((struct &) (struct *) &aps[4]); > ... > > After gimplification, it looks like: > ... > f3 (int i) > { > long intD.5 x.0D.4231; > struct va_listD.4222 apsD.4227[10]; > > try > { > # USE = anything > # CLB = anything > __builtin_va_startD.1052 (&apsD.4227[4], 0); > # USE = anything > # CLB = anything > x.0D.4231 = VA_ARG (&apsD.4227[4], 0B); > apsD.4227[4] = apsD.4227[4]; > xD.4223 = x.0D.4231; > # USE = anything > # CLB = anything > __builtin_va_endD.1051 (&apsD.4227[4]); > } > finally > { > apsD.4227 = {CLOBBER}; > } > } > ... > > The nop 'apsD.4227[4] = apsD.4227[4]' introduced during gimplification is > not meant to be there. > > > There is already a test 'TREE_OPERAND (ap, 0) != TREE_OPERAND (ap_copy, 0))' > in gimplify_modify_expr to prevent this nop: > ... > /* When gimplifying the &ap argument of va_arg, we might end up with > ap.1 = ap > va_arg (&ap.1, 0B) > We need to assign ap.1 back to ap, otherwise va_arg has no effect on > ap. */ > if (ap != NULL_TREE > && TREE_CODE (ap) == ADDR_EXPR > && TREE_CODE (ap_copy) == ADDR_EXPR > && TREE_OPERAND (ap, 0) != TREE_OPERAND (ap_copy, 0)) > gimplify_assign (TREE_OPERAND (ap, 0), TREE_OPERAND (ap_copy, 0), > pre_p); > ... > > But the test is a pointer equality test, and it fails in this case. > > The patches fixes the problem by using operand_equal_p to do the equality > test. > > > Bootstrapped and reg-tested on x86_64. > > Did minimal non-bootstrap build on arm and reg-tested. > > OK for trunk? Hmm, ok for now. But I wonder if we can't fix things to not require that odd extra copy. In fact that we introduce ap.1 looks completely bogus to me (and we don't in this case for arm). Note that the pointer compare obviously fails because we unshare the expression. So ... what breaks if we simply remove this odd "fixup"? Thanks, Richard. > Thanks, > - Tom
Fix va_arg ap_copy nop detection 2015-04-22 Tom de Vries <tom@codesourcery.com> PR tree-optimization/65823 * gimplify.c (gimplify_modify_expr): Use operand_equal_p to test for equality between ap_copy and ap. --- gcc/gimplify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/gimplify.c b/gcc/gimplify.c index 0a8ef84..c68bd47 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -4792,7 +4792,7 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, if (ap != NULL_TREE && TREE_CODE (ap) == ADDR_EXPR && TREE_CODE (ap_copy) == ADDR_EXPR - && TREE_OPERAND (ap, 0) != TREE_OPERAND (ap_copy, 0)) + && !operand_equal_p (TREE_OPERAND (ap, 0), TREE_OPERAND (ap_copy, 0), 0)) gimplify_assign (TREE_OPERAND (ap, 0), TREE_OPERAND (ap_copy, 0), pre_p); if (want_value) -- 1.9.1
Hi, this patch fixes PR65823. The problem is a verify_gimple ICE during compilation of gcc.c-torture/execute/stdarg-2.c for arm at -O0/-O1: ... In function 'f3': src/gcc/testsuite/gcc.c-torture/execute/stdarg-2.c:61:1: error: incorrect sharing of tree nodes aps[4] # .MEM_5 = VDEF <.MEM_11> aps[4] = aps[4]; ... Before gimplification, f3 looks like this in the original dump: ... struct va_list aps[10]; struct va_list aps[10]; __builtin_va_start ((struct &) (struct *) &aps[4], i); x = VA_ARG_EXPR <aps[4]>; __builtin_va_end ((struct &) (struct *) &aps[4]); ... After gimplification, it looks like: ... f3 (int i) { long intD.5 x.0D.4231; struct va_listD.4222 apsD.4227[10]; try { # USE = anything # CLB = anything __builtin_va_startD.1052 (&apsD.4227[4], 0); # USE = anything # CLB = anything x.0D.4231 = VA_ARG (&apsD.4227[4], 0B); apsD.4227[4] = apsD.4227[4]; xD.4223 = x.0D.4231; # USE = anything # CLB = anything __builtin_va_endD.1051 (&apsD.4227[4]); } finally { apsD.4227 = {CLOBBER}; } } ... The nop 'apsD.4227[4] = apsD.4227[4]' introduced during gimplification is not meant to be there. There is already a test 'TREE_OPERAND (ap, 0) != TREE_OPERAND (ap_copy, 0))' in gimplify_modify_expr to prevent this nop: ... /* When gimplifying the &ap argument of va_arg, we might end up with ap.1 = ap va_arg (&ap.1, 0B) We need to assign ap.1 back to ap, otherwise va_arg has no effect on ap. */ if (ap != NULL_TREE && TREE_CODE (ap) == ADDR_EXPR && TREE_CODE (ap_copy) == ADDR_EXPR && TREE_OPERAND (ap, 0) != TREE_OPERAND (ap_copy, 0)) gimplify_assign (TREE_OPERAND (ap, 0), TREE_OPERAND (ap_copy, 0), pre_p); ... But the test is a pointer equality test, and it fails in this case. The patches fixes the problem by using operand_equal_p to do the equality test. Bootstrapped and reg-tested on x86_64. Did minimal non-bootstrap build on arm and reg-tested. OK for trunk? Thanks, - Tom