[Bug rtl-optimization/58726] [4.7/4.8/4.9 Regression] wrong code at -Os on x86_64-linux-gnu (affecting trunk/4.7/4.6, but not 4.8)

["jakub at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58726

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Testcase suitable for gcc.c-torture/execute/:

int a, c;
union { int f1; int f2 : 1; } b;

short
foo (short p)
{
  return p < 0 ? p : a;
}

int
main ()
{
  if (sizeof (short) * __CHAR_BIT__ != 16
      || sizeof (int) * __CHAR_BIT__ != 32)
    return 0;
  b.f1 = 56374;
  unsigned short d;
  int e = b.f2;
  d = e == 0 ? b.f1 : 0;
  c = foo (d);
  if (c != (short) 56374)
    __builtin_abort ();
  return 0;
}

This seems to be a bug in the combiner, during combination of:
(gdb) p debug_rtx (i3)
(insn 22 21 23 2 (parallel [
            (set (reg:CCGOC 17 flags)
                (compare:CCGOC (and:HI (reg/v:HI 83 [ d ])
                        (const_int -9162 [0xffffffffffffdc36]))
                    (const_int 0 [0])))
            (set (reg:HI 85 [ D.1789 ])
                (and:HI (reg/v:HI 83 [ d ])
                    (const_int -9162 [0xffffffffffffdc36])))
        ]) pr58726.c:7 395 {*andhi_2}
     (expr_list:REG_DEAD (reg/v:HI 83 [ d ])
        (nil)))
(gdb) p debug_rtx (i2)
(insn 55 54 56 2 (parallel [
            (set (subreg:SI (reg/v:HI 83 [ d ]) 0)
                (if_then_else:SI (ltu:SI (reg:CC 17 flags)
                        (const_int 0 [0]))
                    (const_int -1 [0xffffffffffffffff])
                    (const_int 0 [0])))
            (clobber (reg:CC 17 flags))
        ]) pr58726.c:19 925 {*x86_movsicc_0_m1}
     (expr_list:REG_DEAD (reg:CC 17 flags)
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))
(gdb) p debug_rtx (i1)
(insn 54 15 55 2 (set (reg:CC 17 flags)
        (compare:CC (reg:QI 98 [ D.1788 ])
            (const_int 1 [0x1]))) pr58726.c:19 5 {*cmpqi_1}
     (expr_list:REG_DEAD (reg:QI 98 [ D.1788 ])
        (nil)))
The problem is that reg:HI 83 is substed multiple twice (with unique_copy == 0)
and apparently force_to_mode seems to destructively modify the expression,
which is shared, around line 8533.  On the second occurrence it is first
force_to_mode'd with mask 0xdc36, and on the first occurrence later on
simplify_comparison performs:
11187          /* If this is a sign bit comparison and we can do arithmetic in
11188         MODE, say that we will only be needing the sign bit of OP0.  */
11189          if (sign_bit_comparison_p && HWI_COMPUTABLE_MODE_P (mode))
11190        op0 = force_to_mode (op0, mode,
11191                     (unsigned HOST_WIDE_INT) 1
11192                     << (GET_MODE_PRECISION (mode) - 1),
11193                     0);
with mask 0x8000, which destructively modifies also the second occurrence.
So the question is, do we have to copy_rtx always just in case, or should some
routines (e.g. force_to_mode) be documented as not modifying the argument in
place?  I guess this issue can be fixed easily just by changing those two
SUBSTs at the end of force_to_mode in IF_THEN_ELSE handling (and there is
another one for ROTATE{,RT}), but the question is if we don't have tons of
similar latent issues.  The changes in place were done using SUBST, so if we
undo_all, they are properly reverted, the problem is when they aren't reverted,
as in this case.