| Message ID | 4F5B0C9F.3010001@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
> So the fix is a one-liner that sets subst_low_luid before the > expand_field_assignment() call. Bootstrapped and tested under i686, > x86-64, powerpc64. Cross-tested on ARM. I was a bit weary that some > optimization regression might appear, which will complicate things, but > everything looks fine. > > I have a larger (customer provided) testcase that exposed this bug after > rev.161655 (the mem-ref2 merge, may be related to effects on bitfields). > So if suitable, please also approve this patch for 4.6/4.7 branches. > > Thanks, > Chung-Lin > > 2012-03-10 Chung-Lin Tang <cltang@codesourcery.com> > > PR rtl-optimization/52528 > * combine.c (can_combine_p): Add setting of subst_low_luid > before call to expand_field_assignment(). OK for mainline, 4.7 branch (once 4.7.0 is released) and 4.6 branch, modulo: + /* The simplification in expand_field_assignment() may call back to + get_last_value(), so set safe guard here. */ + subst_low_luid = DF_INSN_LUID (insn); No () in comments, just use the function name.
Index: combine.c =================================================================== --- combine.c (revision 185168) +++ combine.c (working copy) @@ -1822,6 +1822,10 @@ can_combine_p (rtx insn, rtx i3, rtx pred ATTRIBUT if (set == 0) return 0; + /* The simplification in expand_field_assignment() may call back to + get_last_value(), so set safe guard here. */ + subst_low_luid = DF_INSN_LUID (insn); + set = expand_field_assignment (set); src = SET_SRC (set), dest = SET_DEST (set);
Hi, As described in the PR, a testcase compiled for PowerPC: struct S { unsigned a : 30; unsigned b : 2; }; int foo (int b) { struct S s = {0}; s.b = b; return bar (0x000b0010, 0x00040100ULL, *(unsigned long *)&s); } currently this is compiled to: foo: lis 6,0x4 li 5,0 ori 6,6,256 li 7,0 crxor 6,6,6 b bar Notice the incorrect code generated: no construction of the 1st arg (reg 3), and wrong code for the 3rd arg (reg 7) The problem seems to be in combine, during calls from try_combine() to can_combine_p(): can_combine_p() has a call to expand_field_assignment(), which may call get_last_value() during its simplification operations (through the reg_nonzero_bits_for_combine() hook); not setting subst_low_luid properly affects its correctness. So the fix is a one-liner that sets subst_low_luid before the expand_field_assignment() call. Bootstrapped and tested under i686, x86-64, powerpc64. Cross-tested on ARM. I was a bit weary that some optimization regression might appear, which will complicate things, but everything looks fine. I have a larger (customer provided) testcase that exposed this bug after rev.161655 (the mem-ref2 merge, may be related to effects on bitfields). So if suitable, please also approve this patch for 4.6/4.7 branches. Thanks, Chung-Lin 2012-03-10 Chung-Lin Tang <cltang@codesourcery.com> PR rtl-optimization/52528 * combine.c (can_combine_p): Add setting of subst_low_luid before call to expand_field_assignment().