Patchwork ARM: Fix PR49049

login
register
mail settings
Submitter Bernd Schmidt
Date Oct. 6, 2011, 1:17 p.m.
Message ID <4E8DAA6C.7010402@codesourcery.com>
Download mbox | patch
Permalink /patch/118095/
State New
Headers show

Comments

Bernd Schmidt - Oct. 6, 2011, 1:17 p.m.
This corrects a brain fart in one of my patches last year: I added
another alternative to a subsi for subtraction of a constant. This is
bogus because such an operation should be canonicalized to a PLUS with
the negative constant, Normally that's what happens, and so testing
never showed that the alternative was only half-finished and didn't
work. PR49049 is a testcase where we do end up replacing a REG with a
constant and produce the bad alternative, leading to a crash.

Tested on arm-eabi and committed as obvious. Will do some sanity checks
on 4.6 and commit there as well.


Bernd

Patch

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 179606)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,8 @@ 
+2011-10-06  Bernd Schmidt  <bernds@codesourcery.com>
+
+	PR target/49049
+	* config/arm/arm.md (arm_subsi3_insn): Lose the last alternative.
+
 2011-10-06  Ulrich Weigand  <ulrich.weigand@linaro.org>
 
 	PR target/50305
Index: gcc/testsuite/gcc.c-torture/compile/pr49049.c
===================================================================
--- gcc/testsuite/gcc.c-torture/compile/pr49049.c	(revision 0)
+++ gcc/testsuite/gcc.c-torture/compile/pr49049.c	(revision 0)
@@ -0,0 +1,28 @@ 
+__extension__ typedef unsigned long long int uint64_t;
+
+static int
+sub (int a, int b)
+{
+  return a - b;
+}
+
+static uint64_t
+add (uint64_t a, uint64_t b)
+{
+  return a + b;
+}
+
+int *ptr;
+
+int
+foo (uint64_t arg1, int *arg2)
+{
+  int j;
+  for (; j < 1; j++)
+    {
+      *arg2 |= sub ( sub (sub (j || 1 ^ 0x1, 1), arg1 < 0x1 <=
+						   sub (1, *ptr & j)),
+		     (sub ( j != 1 || sub (j && j, 1) >= 0,
+		       add (!j > arg1, 0x35DLL))));
+    }
+}
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 179606)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@ 
+2011-10-06  Bernd Schmidt  <bernds@codesourcery.com>
+
+	PR target/49049
+	* gcc.c-torture/compile/pr49049.c: New test.
+
 2011-10-06  Ulrich Weigand  <ulrich.weigand@linaro.org>
 
 	PR target/50305
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md	(revision 179606)
+++ gcc/config/arm/arm.md	(working copy)
@@ -1213,27 +1213,24 @@  (define_insn "thumb1_subsi3_insn"
 
 ; ??? Check Thumb-2 split length
 (define_insn_and_split "*arm_subsi3_insn"
-  [(set (match_operand:SI           0 "s_register_operand" "=r,r,rk,r,r")
-	(minus:SI (match_operand:SI 1 "reg_or_int_operand" "rI,r,k,?n,r")
-		  (match_operand:SI 2 "reg_or_int_operand" "r,rI,r, r,?n")))]
+  [(set (match_operand:SI           0 "s_register_operand" "=r,r,rk,r")
+	(minus:SI (match_operand:SI 1 "reg_or_int_operand" "rI,r,k,?n")
+		  (match_operand:SI 2 "reg_or_int_operand" "r,rI,r, r")))]
   "TARGET_32BIT"
   "@
    rsb%?\\t%0, %2, %1
    sub%?\\t%0, %1, %2
    sub%?\\t%0, %1, %2
-   #
    #"
-  "&& ((GET_CODE (operands[1]) == CONST_INT
-       	&& !const_ok_for_arm (INTVAL (operands[1])))
-       || (GET_CODE (operands[2]) == CONST_INT
-	   && !const_ok_for_arm (INTVAL (operands[2]))))"
+  "&& (GET_CODE (operands[1]) == CONST_INT
+       && !const_ok_for_arm (INTVAL (operands[1])))"
   [(clobber (const_int 0))]
   "
   arm_split_constant (MINUS, SImode, curr_insn,
                       INTVAL (operands[1]), operands[0], operands[2], 0);
   DONE;
   "
-  [(set_attr "length" "4,4,4,16,16")
+  [(set_attr "length" "4,4,4,16")
    (set_attr "predicable" "yes")]
 )