Re: [PATCH][ARM] PR target/68648: Fold NOT of CONST_INT in andsi_iorsi3_notsi splitter

[Kyrill Tkachov]

Hi Ramana,

On 15/12/15 21:20, Ramana Radhakrishnan wrote:

On 07/12/15 10:39, Kyrill Tkachov wrote:

Hi all,

In this PR we ICE because during post-reload splitting we generate the insn:
(insn 27 26 11 2 (set (reg:SI 0 r0 [orig:121 D.4992 ] [121])
         (and:SI (not:SI (const_int 1 [0x1]))
             (reg:SI 0 r0 [orig:121 D.4992 ] [121])))
      (nil))


The splitter at fault is andsi_iorsi3_notsi that accepts a const_int in 
operands[3]
and outputs (not (match_dup 3)). It should really be trying to constant fold 
the result
first.  This patch does that by calling simplify_gen_unary to generate the 
complement
of operands[3] if it's a register or the appropriate const_int rtx with the 
correct
folded result that will still properly match the arm bic-immediate instruction.

Bootstrapped and tested on arm-none-eabi.

Is this ok for trunk?

This appears on GCC 4.9 and GCC 5 and I'll be testing the fix there as well.
Ok for those branches if testing is successful?

Thanks,
Kyrill

2015-12-07  Kyrylo Tkachov  <kyrylo.tkac...@arm.com>

     PR target/68648
     * config/arm/arm.md (*andsi_iorsi3_notsi): Try to simplify
     the complement of operands[3] during splitting.

2015-12-07  Kyrylo Tkachov  <kyrylo.tkac...@arm.com>

     PR target/68648
     * gcc.c-torture/execute/pr68648.c: New test.

arm-not-int.patch


diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 
2b48bbaf034b286d723536ec2aa6fe0f9b312911..dfb75c5f11c66c6b4a34ff3071b5a0957c3512cb
 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -3274,8 +3274,22 @@ (define_insn_and_split "*andsi_iorsi3_notsi"
    "#"   ; "orr%?\\t%0, %1, %2\;bic%?\\t%0, %0, %3"
    "&& reload_completed"
    [(set (match_dup 0) (ior:SI (match_dup 1) (match_dup 2)))
-   (set (match_dup 0) (and:SI (not:SI (match_dup 3)) (match_dup 0)))]
-  ""
+   (set (match_dup 0) (and:SI (match_dup 4) (match_dup 5)))]
+  {
+     /* If operands[3] is a constant make sure to fold the NOT into it
+       to avoid creating a NOT of a CONST_INT.  */
+    rtx not_rtx = simplify_gen_unary (NOT, SImode, operands[3], SImode);
+    if (CONST_INT_P (not_rtx))
+      {
+       operands[4] = operands[0];
+       operands[5] = not_rtx;
+      }
+    else
+      {
+       operands[5] = operands[0];
+       operands[4] = not_rtx;
+      }

Watch out indentation here - I'm not sure if this is your mail client or mine 
causing this sort of thing.

Looks fine in two of my editors and also in the svn diff output, so I think 
there
must be something on your end.

Ok with that double checked.

Thanks, I've committed it as r231675.
Will backport to the branches after some time on trunk.

Kyrill

regards
Ramana

+  }
    [(set_attr "length" "8")
     (set_attr "ce_count" "2")
     (set_attr "predicable" "yes")
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68648.c 
b/gcc/testsuite/gcc.c-torture/execute/pr68648.c
new file mode 100644
index 
0000000000000000000000000000000000000000..fc66806a99a0abef7bd517ae2f5200b387e69ce4
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr68648.c
@@ -0,0 +1,20 @@
+int __attribute__ ((noinline))
+foo (void)
+{
+  return 123;
+}
+
+int __attribute__ ((noinline))
+bar (void)
+{
+  int c = 1;
+  c |= 4294967295 ^ (foo () | 4073709551608);
+  return c;
+}
+
+int
+main ()
+{
+  if (bar () != 0x83fd4005)
+    __builtin_abort ();
+}