public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, ARM] correctly encode the CC reg data flow
@ 2016-12-18 13:15 Bernd Edlinger
  2017-01-13 13:50 ` Richard Earnshaw (lists)
  0 siblings, 1 reply; 21+ messages in thread
From: Bernd Edlinger @ 2016-12-18 13:15 UTC (permalink / raw)
  To: gcc-patches
  Cc: Ramana Radhakrishnan, Richard Earnshaw, Kyrill Tkachov, Wilco Dijkstra

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

Hi,

this is related to PR77308, the follow-up patch will depend on this one.

When trying the split the *arm_cmpdi_insn and *arm_cmpdi_unsigned
before reload, a mis-compilation in libgcc function __gnu_satfractdasq
was discovered, see [1] for more details.

The reason seems to be that when the *arm_cmpdi_insn is directly
followed by a *arm_cmpdi_unsigned instruction, both are split
up into this:

   [(set (reg:CC CC_REGNUM)
         (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))))])]

   [(set (reg:CC CC_REGNUM)
         (compare:CC (match_dup 2) (match_dup 3)))
    (cond_exec (eq:SI (reg:CC CC_REGNUM) (const_int 0))
               (set (reg:CC CC_REGNUM)
                    (compare:CC (match_dup 0) (match_dup 1))))]

The problem is that the reg:CC from the *subsi3_carryin_compare
is not mentioning that the reg:CC is also dependent on the reg:CC
from before.  Therefore the *arm_cmpsi_insn appears to be
redundant and thus got removed, because the data values are identical.

I think that applies to a number of similar pattern where data
flow is happening through the CC reg.

So this is a kind of correctness issue, and should be fixed
independently from the optimization issue PR77308.

Therefore I think the patterns need to specify the true
value that will be in the CC reg, in order for cse to
know what the instructions are really doing.


Bootstrapped and reg-tested on arm-linux-gnueabihf.
Is it OK for trunk?


Thanks
Bernd.


[1] https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00680.html

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-pr77308-5.diff --]
[-- Type: text/x-patch; name="patch-pr77308-5.diff", Size: 10117 bytes --]

2016-12-10  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR target/77308
	* config/arm/arm.md (adddi3_compareV, *addsi3_compareV_upper,
	adddi3_compareC, *addsi3_compareC_upper, subdi3_compare1,
	subsi3_carryin_compare, subsi3_carryin_compare_const,
	negdi2_compare, *negsi2_carryin_compare,
	*arm_cmpdi_insn): Fix the CC reg dataflow.

Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md	(revision 243515)
+++ gcc/config/arm/arm.md	(working copy)
@@ -669,17 +669,15 @@
 	      (set (match_dup 0) (plus:SI (match_dup 1) (match_dup 2)))])
    (parallel [(set (reg:CC_V CC_REGNUM)
 		   (ne:CC_V
-		    (plus:DI (plus:DI
-			      (sign_extend:DI (match_dup 4))
-			      (sign_extend:DI (match_dup 5)))
-			     (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
-		    (plus:DI (sign_extend:DI
-			      (plus:SI (match_dup 4) (match_dup 5)))
-			     (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
-	     (set (match_dup 3) (plus:SI (plus:SI
-					  (match_dup 4) (match_dup 5))
-					 (ltu:SI (reg:CC_C CC_REGNUM)
-						 (const_int 0))))])]
+		     (plus:DI (plus:DI (sign_extend:DI (match_dup 4))
+				       (sign_extend:DI (match_dup 5)))
+			      (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
+		    (sign_extend:DI
+		      (plus:SI (plus:SI (match_dup 4) (match_dup 5))
+			       (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))))
+	      (set (match_dup 3) (plus:SI (plus:SI (match_dup 4) (match_dup 5))
+					  (ltu:SI (reg:CC_C CC_REGNUM)
+						  (const_int 0))))])]
   "
   {
     operands[3] = gen_highpart (SImode, operands[0]);
@@ -713,13 +711,13 @@
   [(set (reg:CC_V CC_REGNUM)
 	(ne:CC_V
 	  (plus:DI
-	   (plus:DI
-	    (sign_extend:DI (match_operand:SI 1 "register_operand" "r"))
-	    (sign_extend:DI (match_operand:SI 2 "register_operand" "r")))
-	   (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
-	  (plus:DI (sign_extend:DI
-		    (plus:SI (match_dup 1) (match_dup 2)))
-		   (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
+	    (plus:DI
+	      (sign_extend:DI (match_operand:SI 1 "register_operand" "r"))
+	      (sign_extend:DI (match_operand:SI 2 "register_operand" "r")))
+	    (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
+	  (sign_extend:DI (plus:SI (plus:SI (match_dup 1) (match_dup 2))
+				   (ltu:SI (reg:CC_C CC_REGNUM)
+					   (const_int 0))))))
    (set (match_operand:SI 0 "register_operand" "=r")
 	(plus:SI
 	 (plus:SI (match_dup 1) (match_dup 2))
@@ -748,17 +746,15 @@
 	      (set (match_dup 0) (plus:SI (match_dup 1) (match_dup 2)))])
    (parallel [(set (reg:CC_C CC_REGNUM)
 		   (ne:CC_C
-		    (plus:DI (plus:DI
-			      (zero_extend:DI (match_dup 4))
-			      (zero_extend:DI (match_dup 5)))
-			     (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
-		    (plus:DI (zero_extend:DI
-			      (plus:SI (match_dup 4) (match_dup 5)))
-			     (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
-	     (set (match_dup 3) (plus:SI
-				 (plus:SI (match_dup 4) (match_dup 5))
-				 (ltu:SI (reg:CC_C CC_REGNUM)
-					 (const_int 0))))])]
+		     (plus:DI (plus:DI (zero_extend:DI (match_dup 4))
+				       (zero_extend:DI (match_dup 5)))
+			      (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
+		    (zero_extend:DI
+		      (plus:SI (plus:SI (match_dup 4) (match_dup 5))
+			       (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))))
+	      (set (match_dup 3) (plus:SI (plus:SI (match_dup 4) (match_dup 5))
+					  (ltu:SI (reg:CC_C CC_REGNUM)
+						  (const_int 0))))])]
   "
   {
     operands[3] = gen_highpart (SImode, operands[0]);
@@ -777,17 +773,16 @@
   [(set (reg:CC_C CC_REGNUM)
 	(ne:CC_C
 	  (plus:DI
-	   (plus:DI
-	    (zero_extend:DI (match_operand:SI 1 "register_operand" "r"))
-	    (zero_extend:DI (match_operand:SI 2 "register_operand" "r")))
-	   (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
-	  (plus:DI (zero_extend:DI
-		    (plus:SI (match_dup 1) (match_dup 2)))
-		   (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
+	    (plus:DI
+	      (zero_extend:DI (match_operand:SI 1 "register_operand" "r"))
+	      (zero_extend:DI (match_operand:SI 2 "register_operand" "r")))
+	    (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
+	  (zero_extend:DI
+	    (plus:SI (plus:SI (match_dup 1) (match_dup 2))
+		     (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))))
    (set (match_operand:SI 0 "register_operand" "=r")
-	(plus:SI
-	 (plus:SI (match_dup 1) (match_dup 2))
-	 (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+	(plus:SI (plus:SI (match_dup 1) (match_dup 2))
+		 (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
   "TARGET_32BIT"
   "adcs%?\\t%0, %1, %2"
   [(set_attr "conds" "set")
@@ -1086,8 +1081,8 @@
 })
 
 (define_insn_and_split "subdi3_compare1"
-  [(set (reg:CC CC_REGNUM)
-	(compare:CC
+  [(set (reg:CC_NCV CC_REGNUM)
+	(compare:CC_NCV
 	  (match_operand:DI 1 "register_operand" "r")
 	  (match_operand:DI 2 "register_operand" "r")))
    (set (match_operand:DI 0 "register_operand" "=&r")
@@ -1098,10 +1093,14 @@
   [(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)))])
-   (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))))])]
+   (parallel [(set (reg:CC_C CC_REGNUM)
+		   (compare:CC_C
+		     (zero_extend:DI (match_dup 4))
+		     (plus:DI (zero_extend:DI (match_dup 5))
+			      (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
+	      (set (match_dup 3)
+		   (minus:SI (minus:SI (match_dup 4) (match_dup 5))
+			     (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))])]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);
@@ -1156,13 +1155,15 @@
 )
 
 (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")))
+  [(set (reg:CC_C CC_REGNUM)
+	(compare:CC_C
+	  (zero_extend:DI (match_operand:SI 1 "s_register_operand" "r"))
+	  (plus:DI
+	    (zero_extend:DI (match_operand:SI 2 "s_register_operand" "r"))
+	    (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
    (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))
+		  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
   "TARGET_32BIT"
   "sbcs\\t%0, %1, %2"
   [(set_attr "conds" "set")
@@ -1170,13 +1171,15 @@
 )
 
 (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")))
+  [(set (reg:CC_C CC_REGNUM)
+	(compare:CC_C
+	  (zero_extend:DI (match_operand:SI 1 "reg_or_int_operand" "r"))
+	  (plus:DI
+	    (zero_extend:DI (match_operand:SI 2 "arm_not_operand" "K"))
+	    (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
    (set (match_operand:SI 0 "s_register_operand" "=r")
-        (minus:SI (plus:SI (match_dup 1)
-                           (match_dup 2))
-                  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+	(minus:SI (plus:SI (match_dup 1) (match_dup 2))
+		  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
   "TARGET_32BIT"
   "sbcs\\t%0, %1, #%B2"
   [(set_attr "conds" "set")
@@ -4633,8 +4636,8 @@
 
 
 (define_insn_and_split "negdi2_compare"
-  [(set (reg:CC CC_REGNUM)
-	(compare:CC
+  [(set (reg:CC_NCV CC_REGNUM)
+	(compare:CC_NCV
 	  (const_int 0)
 	  (match_operand:DI 1 "register_operand" "0,r")))
    (set (match_operand:DI 0 "register_operand" "=r,&r")
@@ -4646,8 +4649,12 @@
 		   (compare:CC (const_int 0) (match_dup 1)))
 	      (set (match_dup 0) (minus:SI (const_int 0)
 					   (match_dup 1)))])
-   (parallel [(set (reg:CC CC_REGNUM)
-		   (compare:CC (const_int 0) (match_dup 3)))
+   (parallel [(set (reg:CC_C CC_REGNUM)
+		   (compare:CC_C
+		     (const_int 0)
+		     (plus:DI
+		       (zero_extend:DI (match_dup 3))
+		       (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
 	     (set (match_dup 2)
 		  (minus:SI
 		   (minus:SI (const_int 0) (match_dup 3))
@@ -4705,12 +4712,14 @@
 )
 
 (define_insn "*negsi2_carryin_compare"
-  [(set (reg:CC CC_REGNUM)
-	(compare:CC (const_int 0)
-		    (match_operand:SI 1 "s_register_operand" "r")))
+  [(set (reg:CC_C CC_REGNUM)
+	(compare:CC_C
+	  (const_int 0)
+	  (plus:DI
+	    (zero_extend:DI (match_operand:SI 1 "s_register_operand" "r"))
+	    (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
    (set (match_operand:SI 0 "s_register_operand" "=r")
-	(minus:SI (minus:SI (const_int 0)
-			    (match_dup 1))
+	(minus:SI (minus:SI (const_int 0) (match_dup 1))
 		  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
   "TARGET_ARM"
   "rscs\\t%0, %1, #0"
@@ -7359,12 +7368,15 @@
   "#"   ; "cmp\\t%Q0, %Q1\;sbcs\\t%2, %R0, %R1"
   "&& reload_completed"
   [(set (reg:CC CC_REGNUM)
-        (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 0) (match_dup 1)))
+   (parallel [(set (reg:CC_C CC_REGNUM)
+		   (compare:CC_C
+		     (zero_extend:DI (match_dup 3))
+		     (plus:DI (zero_extend:DI (match_dup 4))
+			      (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
+	      (set (match_dup 2)
+		   (minus:SI (match_dup 5)
+			     (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))])]
   {
     operands[3] = gen_highpart (SImode, operands[0]);
     operands[0] = gen_lowpart (SImode, operands[0]);

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2017-10-10 19:03 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-18 13:15 [PATCH, ARM] correctly encode the CC reg data flow Bernd Edlinger
2017-01-13 13:50 ` Richard Earnshaw (lists)
2017-01-13 16:10   ` Bernd Edlinger
2017-01-13 18:29     ` Bernd Edlinger
2017-01-18 15:43       ` Bernd Edlinger
2017-04-20 19:10         ` [PING] " Bernd Edlinger
2017-04-29 17:32           ` [PING**2] " Bernd Edlinger
2017-05-12 16:50             ` [PING**3] " Bernd Edlinger
2017-06-01 16:01               ` [PING**4] " Bernd Edlinger
     [not found]               ` <eb07f6a9-522b-0497-fc13-f3e4508b8277@hotmail.de>
2017-06-14 12:34                 ` [PING**5] " Bernd Edlinger
     [not found]                 ` <74eaaa44-40f0-4b12-1aec-4b9926158efe@hotmail.de>
2017-07-05 18:11                   ` [PING**6] " Bernd Edlinger
2017-09-04 13:55         ` Kyrill Tkachov
2017-09-04 19:54           ` Bernd Edlinger
     [not found]           ` <a55cfa36-bb99-3433-f99e-c261fbe5dac1@hotmail.de>
2017-09-06 12:44             ` Bernd Edlinger
2017-09-06 12:52               ` Richard Earnshaw (lists)
2017-09-06 13:00                 ` Bernd Edlinger
2017-09-06 13:17                 ` Bernd Edlinger
2017-09-06 15:31                   ` Kyrill Tkachov
2017-09-17  8:38                     ` [PING] " Bernd Edlinger
2017-10-09 13:02                   ` Richard Earnshaw (lists)
2017-10-10 19:11                     ` Bernd Edlinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).