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

Reply via email to