diff mbox series

arm: Fix incorrect modes with 'borrow' operations [PR90311]

Message ID 20200305172929.31838-1-rearnsha@arm.com
State New
Headers show
Series arm: Fix incorrect modes with 'borrow' operations [PR90311] | expand

Commit Message

Richard Earnshaw March 5, 2020, 5:29 p.m. UTC
Looking through the arm backend I noticed that the modes used to pass
comparison types into subtract-with-carry operations were being
incorrectly set.  The result is that the compiler is not truly
self-consistent.  To clean this up I've introduced a new predicate,
arm_borrow_operation (borrowed from the AArch64 backend) which can
match the comparison type with the required mode and then fixed all
the patterns to use this.  The split patterns that were generating
incorrect modes have all obviously been fixed as well.

The basic rule for the use of a borrow is:
- if the condition code was set by a 'subtract-like' operation (subs, cmp),
  then use CCmode and LTU.
- if the condition code was by unsigned overflow of addition (adds), then
  use CC_Cmode and GEU.

gcc:
	PR target/90311
	Backport from master
	* config/arm/predicates.md (arm_borrow_operation): New predicate.
	* config/arm/arm.c (subdi3_compare1): Use CCmode for the split.
	(arm_subdi3, subdi_di_zesidi, subdi_di_sesidi): Likewise.
	(subdi_zesidi_zesidi): Likewise.
	(negdi2_compare, negdi2_insn): Likewise.
	(negdi_extensidi): Likewise.
	(negdi_zero_extendsidi): Likewise.
	(arm_cmpdi_insn): Likewise.
	(subsi3_carryin): Use arm_borrow_operation.
	(subsi3_carryin_const): Likewise.
	(subsi3_carryin_const0): Likewise.
	(subsi3_carryin_compare): Likewise.
	(subsi3_carryin_compare_const): Likewise.
	(subsi3_carryin_compare_const0): Likewise.
	(subsi3_carryin_shift): Likewise.
	(rsbsi3_carryin_shift): Likewise.
	(negsi2_carryin_compare): Likewise.

gcc/testsuite:
2020-03-05  Jakub Jelinek  <jakub@redhat.com>

	Backport from master
	PR target/90311
	* gcc.c-torture/execute/pr90311.c: New test.
---
 gcc/config/arm/arm.md                         | 99 ++++++++++---------
 gcc/config/arm/predicates.md                  | 21 ++++
 gcc/testsuite/gcc.c-torture/execute/pr90311.c | 16 +++
 3 files changed, 87 insertions(+), 49 deletions(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr90311.c
diff mbox series

Patch

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index efa2d31317b..53e54874c12 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -1119,7 +1119,7 @@  (define_insn_and_split "subdi3_compare1"
    (parallel [(set (reg:CC CC_REGNUM)
 		   (compare:CC (match_dup 4) (match_dup 5)))
 	     (set (match_dup 3) (minus:SI (minus:SI (match_dup 4) (match_dup 5))
-			       (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))])]
+			       (ltu:SI (reg:CC CC_REGNUM) (const_int 0))))])]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
@@ -1150,7 +1150,7 @@  (define_insn "*subsi3_carryin"
   [(set (match_operand:SI 0 "s_register_operand" "=r,r,r")
 	(minus:SI (minus:SI (match_operand:SI 1 "reg_or_int_operand" "r,I,Pz")
 			    (match_operand:SI 2 "s_register_operand" "r,r,r"))
-		  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+		  (match_operand:SI 3 "arm_borrow_operation" "")))]
   "TARGET_32BIT"
   "@
    sbc%?\\t%0, %1, %2
@@ -1164,9 +1164,10 @@  (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_neg_immediate_operand" "L"))
-                  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+	(minus:SI (plus:SI
+		   (match_operand:SI 1 "s_register_operand" "r")
+		   (match_operand:SI 2 "arm_neg_immediate_operand" "L"))
+		  (match_operand:SI 3 "arm_borrow_operation" "")))]
   "TARGET_32BIT"
   "sbc\\t%0, %1, #%n2"
   [(set_attr "conds" "use")
@@ -1175,8 +1176,8 @@  (define_insn "*subsi3_carryin_const"
 
 (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))))]
+	(minus:SI (match_operand:SI 1 "s_register_operand" "r")
+		  (match_operand:SI 2 "arm_borrow_operation" "")))]
   "TARGET_32BIT"
   "sbc\\t%0, %1, #0"
   [(set_attr "conds" "use")
@@ -1185,12 +1186,11 @@  (define_insn "*subsi3_carryin_const0"
 
 (define_insn "*subsi3_carryin_compare"
   [(set (reg:CC CC_REGNUM)
-        (compare:CC (match_operand:SI 1 "s_register_operand" "r")
-                    (match_operand:SI 2 "s_register_operand" "r")))
+	(compare:CC (match_operand:SI 1 "s_register_operand" "r")
+		    (match_operand:SI 2 "s_register_operand" "r")))
    (set (match_operand:SI 0 "s_register_operand" "=r")
-        (minus:SI (minus:SI (match_dup 1)
-                            (match_dup 2))
-                  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+	(minus:SI (minus:SI (match_dup 1) (match_dup 2))
+		  (match_operand:SI 3 "arm_borrow_operation" "")))]
   "TARGET_32BIT"
   "sbcs\\t%0, %1, %2"
   [(set_attr "conds" "set")
@@ -1199,12 +1199,13 @@  (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 "const_int_I_operand" "I")))
+	(compare:CC (match_operand:SI 1 "reg_or_int_operand" "r")
+		    (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_operand:SI 3 "arm_neg_immediate_operand" "L"))
-                  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+	(minus:SI (plus:SI
+		   (match_dup 1)
+		   (match_operand:SI 3 "arm_neg_immediate_operand" "L"))
+		  (match_operand:SI 4 "arm_borrow_operation" "")))]
   "TARGET_32BIT
    && (INTVAL (operands[2])
        == trunc_int_for_mode (-INTVAL (operands[3]), SImode))"
@@ -1215,11 +1216,11 @@  (define_insn "*subsi3_carryin_compare_const"
 
 (define_insn "*subsi3_carryin_compare_const0"
   [(set (reg:CC CC_REGNUM)
-        (compare:CC (match_operand:SI 1 "reg_or_int_operand" "r")
+	(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))))]
+	(minus:SI (match_dup 1)
+		  (match_operand:SI 2 "arm_borrow_operation" "")))]
   "TARGET_32BIT"
   "sbcs\\t%0, %1, #0"
   [(set_attr "conds" "set")
@@ -1229,28 +1230,28 @@  (define_insn "*subsi3_carryin_compare_const0"
 (define_insn "*subsi3_carryin_shift"
   [(set (match_operand:SI 0 "s_register_operand" "=r")
 	(minus:SI (minus:SI
-		  (match_operand:SI 1 "s_register_operand" "r")
-                  (match_operator:SI 2 "shift_operator"
-                   [(match_operand:SI 3 "s_register_operand" "r")
-                    (match_operand:SI 4 "reg_or_int_operand" "rM")]))
-                 (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+		   (match_operand:SI 1 "s_register_operand" "r")
+		   (match_operator:SI 2 "shift_operator"
+		    [(match_operand:SI 3 "s_register_operand" "r")
+		     (match_operand:SI 4 "reg_or_int_operand" "rM")]))
+		  (match_operand:SI 5 "arm_borrow_operation" "")))]
   "TARGET_32BIT"
   "sbc%?\\t%0, %1, %3%S2"
   [(set_attr "conds" "use")
    (set_attr "predicable" "yes")
    (set (attr "type") (if_then_else (match_operand 4 "const_int_operand" "")
-		      (const_string "alu_shift_imm")
-                     (const_string "alu_shift_reg")))]
+				    (const_string "alu_shift_imm")
+				    (const_string "alu_shift_reg")))]
 )
 
 (define_insn "*rsbsi3_carryin_shift"
   [(set (match_operand:SI 0 "s_register_operand" "=r")
 	(minus:SI (minus:SI
-                  (match_operator:SI 2 "shift_operator"
-                   [(match_operand:SI 3 "s_register_operand" "r")
-                    (match_operand:SI 4 "reg_or_int_operand" "rM")])
+		   (match_operator:SI 2 "shift_operator"
+		    [(match_operand:SI 3 "s_register_operand" "r")
+		     (match_operand:SI 4 "reg_or_int_operand" "rM")])
 		   (match_operand:SI 1 "s_register_operand" "r"))
-                 (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+		  (match_operand:SI 5 "arm_borrow_operation" "")))]
   "TARGET_ARM"
   "rsc%?\\t%0, %1, %3%S2"
   [(set_attr "conds" "use")
@@ -1320,7 +1321,7 @@  (define_insn_and_split "*arm_subdi3"
 		   (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 (minus:SI (match_dup 4) (match_dup 5))
-			       (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+			       (ltu:SI (reg:CC CC_REGNUM) (const_int 0))))]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
@@ -1347,7 +1348,7 @@  (define_insn_and_split "*subdi_di_zesidi"
 		   (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 (match_dup 4)
-                                (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+				(ltu:SI (reg:CC CC_REGNUM) (const_int 0))))]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
@@ -1374,7 +1375,7 @@  (define_insn_and_split "*subdi_di_sesidi"
    (set (match_dup 3) (minus:SI (minus:SI (match_dup 4)
                                          (ashiftrt:SI (match_dup 2)
                                                       (const_int 31)))
-                                (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+                                (ltu:SI (reg:CC CC_REGNUM) (const_int 0))))]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
@@ -1401,7 +1402,7 @@  (define_insn_and_split "*subdi_zesidi_di"
 		   (compare:CC (match_dup 2) (match_dup 1)))
 	      (set (match_dup 0) (minus:SI (match_dup 2) (match_dup 1)))])
    (set (match_dup 3) (minus:SI (minus:SI (const_int 0) (match_dup 4))
-			       (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+			       (ltu:SI (reg:CC CC_REGNUM) (const_int 0))))]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
@@ -1431,7 +1432,7 @@  (define_insn_and_split "*subdi_sesidi_di"
                                 (ashiftrt:SI (match_dup 2)
                                              (const_int 31))
                                 (match_dup 4))
-			       (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+			       (ltu:SI (reg:CC CC_REGNUM) (const_int 0))))]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
@@ -1457,7 +1458,7 @@  (define_insn_and_split "*subdi_zesidi_zesidi"
 		   (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 (minus:SI (match_dup 1) (match_dup 1))
-			       (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+			       (ltu:SI (reg:CC CC_REGNUM) (const_int 0))))]
   {
        operands[3] = gen_highpart (SImode, operands[0]);
        operands[0] = gen_lowpart (SImode, operands[0]);
@@ -4670,7 +4671,7 @@  (define_insn_and_split "negdi2_compare"
 	     (set (match_dup 2)
 		  (minus:SI
 		   (minus:SI (const_int 0) (match_dup 3))
-		   (ltu:SI (reg:CC_C CC_REGNUM)
+		   (ltu:SI (reg:CC CC_REGNUM)
 			   (const_int 0))))])]
   {
     operands[2] = gen_highpart (SImode, operands[0]);
@@ -4712,7 +4713,7 @@  (define_insn_and_split "*negdi2_insn"
 		   (compare:CC (const_int 0) (match_dup 1)))
 	      (set (match_dup 0) (minus:SI (const_int 0) (match_dup 1)))])
    (set (match_dup 2) (minus:SI (minus:SI (const_int 0) (match_dup 3))
-                                (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+                                (ltu:SI (reg:CC CC_REGNUM) (const_int 0))))]
   {
     operands[2] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
@@ -4731,7 +4732,7 @@  (define_insn "*negsi2_carryin_compare"
    (set (match_operand:SI 0 "s_register_operand" "=r")
 	(minus:SI (minus:SI (const_int 0)
 			    (match_dup 1))
-		  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+		  (match_operand:SI 2 "arm_borrow_operation" "")))]
   "TARGET_ARM"
   "rscs\\t%0, %1, #0"
   [(set_attr "conds" "set")
@@ -4808,7 +4809,7 @@  (define_insn_and_split "*negdi_extendsidi"
 		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);
+	rtx cc_reg = gen_rtx_REG (CCmode, CC_REGNUM);
 
 	emit_insn (gen_rtx_SET (high,
 				gen_rtx_ASHIFTRT (SImode, operands[1],
@@ -4870,10 +4871,10 @@  (define_insn_and_split "*negdi_zero_extendsidi"
       ;; since we just need to propagate the carry.
   "&& reload_completed"
   [(parallel [(set (reg:CC CC_REGNUM)
-                   (compare:CC (const_int 0) (match_dup 1)))
-              (set (match_dup 0) (minus:SI (const_int 0) (match_dup 1)))])
+		   (compare:CC (const_int 0) (match_dup 1)))
+	      (set (match_dup 0) (minus:SI (const_int 0) (match_dup 1)))])
    (set (match_dup 2) (minus:SI (minus:SI (match_dup 2) (match_dup 2))
-                                (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+				(ltu:SI (reg:CC CC_REGNUM) (const_int 0))))]
   {
     operands[2] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
@@ -7454,12 +7455,12 @@  (define_insn_and_split "*arm_cmpdi_insn"
   "#"   ; "cmp\\t%Q0, %Q1\;sbcs\\t%2, %R0, %R1"
   "&& reload_completed"
   [(set (reg:CC CC_REGNUM)
-        (compare:CC (match_dup 0) (match_dup 1)))
+	(compare:CC (match_dup 0) (match_dup 1)))
    (parallel [(set (reg:CC CC_REGNUM)
-                   (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))))])]
+		   (compare:CC (match_dup 3) (match_dup 4)))
+	      (set (match_dup 2)
+		   (minus:SI (match_dup 5)
+			     (ltu:SI (reg:CC CC_REGNUM) (const_int 0))))])]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
index f53378a250e..25f86471ded 100644
--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -358,6 +358,27 @@  (define_predicate "arm_comparison_operator_mode"
 (define_special_predicate "lt_ge_comparison_operator"
   (match_code "lt,ge"))
 
+;; Match a "borrow" operation for use with SBC.  The precise code will
+;; depend on the form of the comparison.  This is generally the inverse of
+;; a carry operation, since the logic of SBC uses "not borrow" in it's
+;; calculation.
+(define_special_predicate "arm_borrow_operation"
+  (match_code "geu,ltu")
+  {
+    if (XEXP (op, 1) != const0_rtx)
+      return false;
+    rtx op0 = XEXP (op, 0);
+    if (!REG_P (op0) || REGNO (op0) != CC_REGNUM)
+      return false;
+    machine_mode ccmode = GET_MODE (op0);
+    if (ccmode == CC_Cmode)
+      return GET_CODE (op) == GEU;
+    else if (ccmode == CCmode)
+      return GET_CODE (op) == LTU;
+    return false;
+  }
+)
+
 ;; The vsel instruction only accepts the ARM condition codes listed below.
 (define_special_predicate "arm_vsel_comparison_operator"
   (and (match_operand 0 "expandable_comparison_operator")
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr90311.c b/gcc/testsuite/gcc.c-torture/execute/pr90311.c
new file mode 100644
index 00000000000..af8cde6d0f0
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr90311.c
@@ -0,0 +1,16 @@ 
+/* PR rtl-optimization/90311 */
+
+int a, b;
+
+int
+main ()
+{
+  unsigned long long x;
+  unsigned int c;
+  __builtin_add_overflow ((unsigned char) a, b, &c);
+  b -= c < (unsigned char) a;
+  x = b;
+  if (x)
+    __builtin_abort ();
+  return 0;
+}