This testcase is miscompiled by g++ -O2 on ia64: ----------------------------------------------- typedef long int intptr_t; struct A { void* mem1; void* mem2; }; static int off1() { return (intptr_t)(&(((struct A*)0x0)->mem1) ); } static int off2() { return (intptr_t)(&(((struct A*)0x0)->mem2) ); } int func( int length) { int off = off2() + 8; if( length ) off = off1() + 4; if( 4 == (off & 4) ) off += 4; return off; } extern "C" void abort (void); int main(void) { if (func(0) == 0) abort(); } -----------------------------------------------
The two functions must be a bit complicated that not already the tree optimizers optimize everything to a constant. For the same reason __builtin_offsetof also hides the bug. The C compiler also optimizes those two functions too early, so the code as it goes into the combiner is already Okay. But with g++ you can see the error in combine. The problem is, that the following instructions: (insn 60 12 31 0 (set (reg/v:SI 341 [ off ]) (if_then_else:SI (ne (reg:BI 344) (const_int 0 [0x0])) (const_int 4 [0x4]) (const_int 16 [0x10]))) 249 {*cmovsi_internal} (expr_list:REG_DEAD (reg:BI 344) (insn 31 60 32 0 (set (reg:DI 345) (and:DI (subreg:DI (reg/v:SI 341 [ off ]) 0) (const_int 4 [0x4]))) 228 {anddi3} (insn 32 31 33 0 (set (reg:SI 346) (subreg:SI (reg:DI 345) 0)) 4 {*movsi_internal} (expr_list:REG_DEAD (reg:DI 345) (insn 33 32 34 0 (set (reg:BI 347) (eq:BI (reg:SI 346) (const_int 0 [0x0]))) 233 {*cmpsi_normal} (expr_list:REG_DEAD (reg:SI 346) in endeffect are combined into: (insn 33 32 34 0 (set (reg/v:SI 341 [ off ]) (if_then_else:SI (ne (reg:BI 344) (const_int 0 [0x0])) (const_int 4 [0x4]) (const_int 0 [0x0]))) 249 {*cmovsi_internal} Note especially that pseudo 341 was not dead in the first sequence and it's value is indeed used later (it's simply returned). Without that combination r341 will have values {4,16}, after that {4,0}. What the combiner tries to do, is to combine that conditional set and that (and 4), which indeed would correctly result in the above {4,0} conditional set. The problem is, that an instruction is created which modifies the r341, which it shouldn't. I debugged this a bit and it's not a simple problem of combine not noticing that r341 is necessary later (it recognizes that), but instead it's an RTL sharing problem in subst(). Reading the code a bit I think combine.c has multiple RTL sharing problems, but here is the issue at hand: try_combine() is called with these instructions in the call which finally does the invalid transformation: i1: (insn 60 12 31 0 (set (reg/v:SI 341 [ off ]) (if_then_else:SI (ne (reg:BI 344) (const_int 0 [0x0])) (const_int 4 [0x4]) (const_int 16 [0x10]))) i2: (insn 33 32 34 0 (set (reg:BI 347) (eq:BI (zero_extract:DI (subreg:DI (reg/v:SI 341 [ off ]) 0) (const_int 1 [0x1]) (const_int 2 [0x2])) (const_int 0 [0x0]))) i3: (jump_insn 34 33 36 0 (set (pc) (if_then_else (ne (reg:BI 347) (const_int 0 [0x0])) (label_ref 39) (pc))) 242 {*br_true} In the course of it's action to modify i3, it will somewhen have modified i1 (!) into: (insn 60 12 31 0 (set (reg/v:SI 341 [ off ]) (if_then_else:SI (ne (reg:BI 344) (const_int 0 [0x0])) (const_int 4 [0x4]) (const_int 0 [0x0]))) This is because i1src is given uncopied to subst(). Anyway, i1src starts as expected as (if_then_else:SI (ne (reg:BI 344) (const_int 0 [0x0])) (const_int 4 [0x4]) (const_int 16 [0x10])) Then, in line 2216 it will be used in a subst call: /* If we already got a failure, don't try to do more. Otherwise, try to substitute in I1 if we have it. */ if (i1 && GET_CODE (newpat) != CLOBBER) { /* Before we can do this substitution, we must redo the test done above (see detailed comments there) that ensures that I1DEST isn't mentioned in any SETs in NEWPAT that are field assignments. */ if (! combinable_i3pat (NULL_RTX, &newpat, i1dest, NULL_RTX, 0, (rtx*) 0)) { undo_all (); return 0; } n_occurrences = 0; subst_low_cuid = INSN_CUID (i1); newpat = subst (newpat, i1dest, i1src, 0, 0); substed_i1 = 1; } What combine tries to do, is also to fold i1 into the resulting instruction which it is in the process of construction. i1src still points right into the i1 pattern, i.e. is not a copy of it. The problem is, that in some situations subst() not only changes the pattern where it substitutes something into, but also the TO argument. I'm fairly sure that this was not as initially designed. But the code in subst does not prevent this. We have: subst (x, from, to) (walk over XEXPs of x) if (COMBINE_RTX_EQUAL_P (XEXP (x, i), from)) .... new = (unique_copy && n_occurrences ? copy_rtx (to) : to); SUBST (XEXP (x, i), new); x = combine_simplify_rtx (x); Note how the 'to' parameter is directly set into the 'x' expression. Even if unique_copy is set this will be prevented only for the second and further substitutions. This means, that now 'x' and 'i1' (from the caller try_combine) share the i1src expression. Now combine_simplify_rtx() comes (some levels up in this recursive invocation of subst()) and tramples all over x, in the process changing something inside the i1src, thereby also changing i1 invalidly. Boom. Now, I have looked at the other calls of subst() and I don't see how they in turn prevent such thing from happening. If they don't use the non-problematic pc_rtx they happily us i2src, i1src or 'from' (in simplify_if_then_else()) potentially changing i2, i1 or something entirely different behind the back. There are some copy_rtx calls but they don't seem to have to do with this special behaviour of 'subst'. Especially the copying of i2pat seems unrelated and actually is protecting against wanted side effects of subst. -- Summary: combine miscompiles Product: gcc Version: 4.1.1 Status: UNCONFIRMED Severity: normal Priority: P3 Component: rtl-optimization AssignedTo: unassigned at gcc dot gnu dot org ReportedBy: matz at gcc dot gnu dot org GCC host triplet: ia64-linux-gnu http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27761