Patchwork [ARM] Fix PR target/59216 incorrect optimization of neg(s_ex:DI())

login
register
mail settings
Submitter Richard Earnshaw
Date Nov. 22, 2013, 3:50 p.m.
Message ID <528F7D3A.1000600@arm.com>
Download mbox | patch
Permalink /patch/293505/
State New
Headers show

Comments

Richard Earnshaw - Nov. 22, 2013, 3:50 p.m.
In PR target/59216 we have a case where the compiler generates incorrect
code for (neg:DI (sign_extend:DI (reg:SI))).  This splitter pattern
generates the wrong output when the register contains INT_MIN.

The shortest sequence we can use here is three insns, but since there
are cases where not having the splitter pattern might result in 4, it's
still beneficial to keep the pattern and to split later, once we know
the register allocation.

Bootstrapped/tested on arm-linux-gnueabihi.

        PR target/59216

gcc/
        * arm.md (negdi_extendsidi): Fix invalid split.

gcc/testsuite/
        * gcc.target/arm/negdi-4.c: Delete invalid test.
        * gcc.dg/torture/pr59216.c: New test.

Applied to trunk.

R.

Patch

Index: gcc/testsuite/gcc.target/arm/negdi-4.c
===================================================================
--- gcc/testsuite/gcc.target/arm/negdi-4.c	(revision 205116)
+++ gcc/testsuite/gcc.target/arm/negdi-4.c	(working copy)
@@ -1,16 +0,0 @@ 
-/* { dg-do compile } */
-/* { dg-require-effective-target arm32 } */
-/* { dg-options "-O2" } */
-
-signed long long negdi_extendsidi (signed int x)
-{
-  return -((signed long long) x);
-}
-/*
-Expected output:
-        rsbs    r0, r0, #0
-        mov     r1, r0, asr #31
-*/
-/* { dg-final { scan-assembler-times "rsb" 1 } } */
-/* { dg-final { scan-assembler-times "asr" 1 } } */
-/* { dg-final { scan-assembler-times "rsc" 0 } } */
Index: gcc/testsuite/gcc.dg/torture/pr59216.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr59216.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr59216.c	(revision 0)
@@ -0,0 +1,32 @@ 
+/* { dg-do run } */
+
+#include <limits.h>
+
+extern void abort (void);
+extern void exit (int);
+
+long long __attribute__((noinline)) f(int a)
+{
+  return -(long long) a;
+}
+
+int
+main()
+{
+  if (f(0) != 0)
+    abort ();
+
+  if (f(1) != -(long long)1)
+    abort ();
+
+  if (f(-1) != -(long long)-1)
+    abort ();
+
+  if (f(INT_MIN) != -(long long)INT_MIN)
+    abort ();
+
+  if (f(INT_MAX) != -(long long)INT_MAX)
+    abort ();
+
+  exit (0);
+}
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md	(revision 205116)
+++ gcc/config/arm/arm.md	(working copy)
@@ -4710,47 +4710,74 @@  (define_expand "negdf2"
 
 ;; Negate an extended 32-bit value.
 (define_insn_and_split "*negdi_extendsidi"
-  [(set (match_operand:DI 0 "s_register_operand" "=r,&r,l,&l")
-	(neg:DI (sign_extend:DI (match_operand:SI 1 "s_register_operand" "0,r,0,l"))))
+  [(set (match_operand:DI 0 "s_register_operand" "=l,r")
+	(neg:DI (sign_extend:DI
+		 (match_operand:SI 1 "s_register_operand" "l,r"))))
    (clobber (reg:CC CC_REGNUM))]
   "TARGET_32BIT"
-  "#" ; rsb\\t%Q0, %1, #0\;asr\\t%R0, %Q0, #31
+  "#"
   "&& reload_completed"
   [(const_int 0)]
   {
-     operands[2] = gen_highpart (SImode, operands[0]);
-     operands[0] = gen_lowpart (SImode, operands[0]);
-     rtx tmp = gen_rtx_SET (VOIDmode,
-                            operands[0],
-                            gen_rtx_MINUS (SImode,
-                                           const0_rtx,
-                                           operands[1]));
-     if (TARGET_ARM)
-       {
-         emit_insn (tmp);
-       }
-     else
-       {
-         /* Set the flags, to emit the short encoding in Thumb2.  */
-         rtx flags = gen_rtx_SET (VOIDmode,
-                                  gen_rtx_REG (CCmode, CC_REGNUM),
-                                  gen_rtx_COMPARE (CCmode,
-                                                   const0_rtx,
-                                                   operands[1]));
-         emit_insn (gen_rtx_PARALLEL (VOIDmode,
-                                      gen_rtvec (2,
-                                                 flags,
-                                                 tmp)));
-       }
-       emit_insn (gen_rtx_SET (VOIDmode,
-                              operands[2],
-                              gen_rtx_ASHIFTRT (SImode,
-                                                operands[0],
-                                                GEN_INT (31))));
-     DONE;
+    rtx low = gen_lowpart (SImode, operands[0]);
+    rtx high = gen_highpart (SImode, operands[0]);
+
+    if (reg_overlap_mentioned_p (low, operands[1]))
+      {
+	/* Input overlaps the low word of the output.  Use:
+		asr	Rhi, Rin, #31
+		rsbs	Rlo, Rin, #0
+		rsc	Rhi, Rhi, #0 (thumb2: sbc Rhi, Rhi, Rhi, lsl #1).  */
+	rtx cc_reg = gen_rtx_REG (CC_Cmode, CC_REGNUM);
+
+	emit_insn (gen_rtx_SET (VOIDmode, high,
+				gen_rtx_ASHIFTRT (SImode, operands[1],
+						  GEN_INT (31))));
+
+	emit_insn (gen_subsi3_compare (low, const0_rtx, operands[1]));
+	if (TARGET_ARM)
+	  emit_insn (gen_rtx_SET (VOIDmode, high,
+				  gen_rtx_MINUS (SImode,
+						 gen_rtx_MINUS (SImode,
+								const0_rtx,
+								high),
+						 gen_rtx_LTU (SImode,
+							      cc_reg,
+							      const0_rtx))));
+	else
+	  {
+	    rtx two_x = gen_rtx_ASHIFT (SImode, high, GEN_INT (1));
+	    emit_insn (gen_rtx_SET (VOIDmode, high,
+				    gen_rtx_MINUS (SImode,
+						   gen_rtx_MINUS (SImode,
+								  high,
+								  two_x),
+						   gen_rtx_LTU (SImode,
+								cc_reg,
+								const0_rtx))));
+	  }
+      }
+    else
+      {
+	/* No overlap, or overlap on high word.  Use:
+		rsb	Rlo, Rin, #0
+		bic	Rhi, Rlo, Rin
+		asr	Rhi, Rhi, #31
+	   Flags not needed for this sequence.  */
+	emit_insn (gen_rtx_SET (VOIDmode, low,
+				gen_rtx_NEG (SImode, operands[1])));
+	emit_insn (gen_rtx_SET (VOIDmode, high,
+				gen_rtx_AND (SImode,
+					     gen_rtx_NOT (SImode, operands[1]),
+					     low)));
+	emit_insn (gen_rtx_SET (VOIDmode, high,
+				gen_rtx_ASHIFTRT (SImode, high,
+						  GEN_INT (31))));
+      }
+    DONE;
   }
-  [(set_attr "length" "8,8,4,4")
-   (set_attr "arch" "a,a,t2,t2")
+  [(set_attr "length" "12")
+   (set_attr "arch" "t2,*")
    (set_attr "type" "multiple")]
 )