Hi!

As the testcase shows, *subsi3_carryin_{const,compare_const} patterns
don't express in RTL what they are actually doing, which may (on the
testcase) does cause miscompilation if we manage to propagate constants
into it or for other reason simplify-rtx.c etc. tries to simplify them,
or if combiner matches them for how they look like.

The *subsi3_carryin{,_compare} patterns look correct, the reason why the
_const patterns want to be different is that the canonical form of
(minus (reg) (const_int N)) is actually (plus (reg) (const_int -N));
the patterns were actually implementing (plus (reg) (const_int ~N))
which is off-by-one.  I had to fix up also two splitters that were
generating these.  Finally, (plus (reg) (const_int 0)) is not canonical,
it should be just (reg), but it is pretty widely used case,
so I've added two simpler patterns for those (the const0 ones).

Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?

2019-02-24  Jakub Jelinek  <ja...@redhat.com>

        PR target/89434
        * config/arm/arm.md (*subsi3_carryin_const): Use
        arm_neg_immediate_operand predicate instead of
        arm_not_immediate_operand, "L" constraint instead of "K" and
        print it using %n2 instead of %B2.
        (*subsi3_carryin_const0): New define_insn.
        (*subsi3_carryin_compare_const): Use const_int_I_operand predicate
        instead of arm_not_operand and "I" constraint instead of "K" and
        print it using %n3 instead of %B2.  Instead of using match_dup 2 add
        another match_operand and in the condition check that it is negation
        of operands[2].
        (*subsi3_carryin_compare_const0): New define_ins.
        (*subdi_di_zesidi): Adjust to use *subsi3_carryin_const0 instead of
        *subsi3_carryin_const.
        (*arm_cmpdi_insn): Fix splitting into *subsi3_carryin_compare_const,
        split into *subsi3_carryin_compare_const0 if the highpart is zero.

        * gcc.c-torture/execute/pr89434.c: New test.

--- gcc/config/arm/arm.md.jj    2019-02-22 15:22:16.034999035 +0100
+++ gcc/config/arm/arm.md       2019-02-23 12:10:47.079659675 +0100
@@ -1145,10 +1145,20 @@ (define_insn "*subsi3_carryin"
 (define_insn "*subsi3_carryin_const"
   [(set (match_operand:SI 0 "s_register_operand" "=r")
         (minus:SI (plus:SI (match_operand:SI 1 "s_register_operand" "r")
-                           (match_operand:SI 2 "arm_not_immediate_operand" 
"K"))
+                           (match_operand:SI 2 "arm_neg_immediate_operand" 
"L"))
                   (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
   "TARGET_32BIT"
-  "sbc\\t%0, %1, #%B2"
+  "sbc\\t%0, %1, #%n2"
+  [(set_attr "conds" "use")
+   (set_attr "type" "adc_imm")]
+)
+
+(define_insn "*subsi3_carryin_const0"
+  [(set (match_operand:SI 0 "s_register_operand" "=r")
+        (minus:SI (match_operand:SI 1 "s_register_operand" "r")
+                  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+  "TARGET_32BIT"
+  "sbc\\t%0, %1, #0"
   [(set_attr "conds" "use")
    (set_attr "type" "adc_imm")]
 )
@@ -1170,13 +1180,26 @@ (define_insn "*subsi3_carryin_compare"
 (define_insn "*subsi3_carryin_compare_const"
   [(set (reg:CC CC_REGNUM)
         (compare:CC (match_operand:SI 1 "reg_or_int_operand" "r")
-                    (match_operand:SI 2 "arm_not_operand" "K")))
+                    (match_operand:SI 2 "const_int_I_operand" "I")))
    (set (match_operand:SI 0 "s_register_operand" "=r")
         (minus:SI (plus:SI (match_dup 1)
-                           (match_dup 2))
+                           (match_operand:SI 3 "arm_neg_immediate_operand" 
"L"))
+                  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+  "TARGET_32BIT && UINTVAL (operands[2]) == -UINTVAL (operands[3])"
+  "sbcs\\t%0, %1, #%n3"
+  [(set_attr "conds" "set")
+   (set_attr "type" "adcs_imm")]
+)
+
+(define_insn "*subsi3_carryin_compare_const0"
+  [(set (reg:CC CC_REGNUM)
+        (compare:CC (match_operand:SI 1 "reg_or_int_operand" "r")
+                   (const_int 0)))
+   (set (match_operand:SI 0 "s_register_operand" "=r")
+        (minus:SI (match_dup 1)
                   (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
   "TARGET_32BIT"
-  "sbcs\\t%0, %1, #%B2"
+  "sbcs\\t%0, %1, #0"
   [(set_attr "conds" "set")
    (set_attr "type" "adcs_imm")]
 )
@@ -1301,14 +1324,13 @@ (define_insn_and_split "*subdi_di_zesidi
   [(parallel [(set (reg:CC CC_REGNUM)
                   (compare:CC (match_dup 1) (match_dup 2)))
              (set (match_dup 0) (minus:SI (match_dup 1) (match_dup 2)))])
-   (set (match_dup 3) (minus:SI (plus:SI (match_dup 4) (match_dup 5))
+   (set (match_dup 3) (minus:SI (match_dup 4)
                                 (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
     operands[4] = gen_highpart (SImode, operands[1]);
     operands[1] = gen_lowpart (SImode, operands[1]);
-    operands[5] = GEN_INT (~0);
    }
   [(set_attr "conds" "clob")
    (set_attr "length" "8")
@@ -7423,16 +7445,19 @@ (define_insn_and_split "*arm_cmpdi_insn"
                    (compare:CC (match_dup 3) (match_dup 4)))
               (set (match_dup 2)
                    (minus:SI (match_dup 5)
-                            (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))])]
+                            (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))])]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
     if (CONST_INT_P (operands[1]))
       {
-        operands[4] = GEN_INT (~INTVAL (gen_highpart_mode (SImode,
-                                                           DImode,
-                                                           operands[1])));
-        operands[5] = gen_rtx_PLUS (SImode, operands[3], operands[4]);
+       operands[4] = gen_highpart_mode (SImode, DImode, operands[1]);
+       if (operands[4] == const0_rtx)
+         operands[5] = operands[3];
+       else
+         operands[5] = gen_rtx_PLUS (SImode, operands[3],
+                                     gen_int_mode (-UINTVAL (operands[4]),
+                                                   SImode));
       }
     else
       {
--- gcc/testsuite/gcc.c-torture/execute/pr89434.c.jj    2019-02-23 
11:41:43.291695725 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr89434.c       2019-02-23 
11:41:43.291695725 +0100
@@ -0,0 +1,29 @@
+/* PR target/89434 */
+
+#if __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8 && __CHAR_BIT__ == 8
+long g = 0;
+
+static inline unsigned long long
+foo (unsigned long long u)
+{
+  unsigned x;
+  __builtin_mul_overflow (-1, g, &x);
+  u |= (unsigned) u < (unsigned short) x;
+  return x - u;
+}
+
+int
+main ()
+{
+  unsigned long long x = foo (0x222222222ULL);
+  if (x != 0xfffffffddddddddeULL)
+    __builtin_abort ();
+  return 0;
+}
+#else
+int
+main ()
+{
+  return 0;
+}
+#endif
--- gcc/testsuite/gcc.dg/pr89434.c.jj   2019-02-23 11:41:43.291695725 +0100
+++ gcc/testsuite/gcc.dg/pr89434.c      2019-02-23 11:41:43.291695725 +0100
@@ -0,0 +1,5 @@
+/* PR target/89434 */
+/* { dg-do run } */
+/* { dg-options "-Og" } */
+
+#include "../gcc.c-torture/execute/pr89434.c"

        Jakub

Reply via email to