Re: [PATCH] ARM: Don't clobber CC reg when it is live after the peephole window

[Richard Earnshaw <rearnsha@arm.com>]

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

On 18/06/13 17:22, Meador Inge wrote:
> Ping.
>
> On 06/06/2013 01:23 PM, Meador Inge wrote:
>> On 06/06/2013 08:11 AM, Richard Earnshaw wrote:
>>
>>> I understand (and agree with) this bit...
>>>
>>>> +(define_peephole2
>>>> +  [(set (reg:CC CC_REGNUM)
>>>> +    (compare:CC (match_operand:SI 0 "register_operand" "")
>>>> +            (match_operand:SI 1 "arm_rhs_operand" "")))
>>>> +   (cond_exec (ne (reg:CC CC_REGNUM) (const_int 0))
>>>> +          (set (match_operand:SI 2 "register_operand" "") (const_int 0)))
>>>> +   (cond_exec (eq (reg:CC CC_REGNUM) (const_int 0))
>>>> +          (set (match_dup 2) (const_int 1)))
>>>> +   (match_scratch:SI 3 "r")]
>>>> +  "TARGET_32BIT && !peep2_reg_dead_p (3, operands[0])"
>>>> +  [(set (match_dup 3) (minus:SI (match_dup 0) (match_dup 1)))
>>>> +   (parallel
>>>> +    [(set (reg:CC CC_REGNUM)
>>>> +      (compare:CC (const_int 0) (match_dup 3)))
>>>> +     (set (match_dup 2) (minus:SI (const_int 0) (match_dup 3)))])
>>>> +   (set (match_dup 2)
>>>> +    (plus:SI (plus:SI (match_dup 2) (match_dup 3))
>>>> +         (geu:SI (reg:CC CC_REGNUM) (const_int 0))))])
>>>> +
>>>
>>> ... but what's this bit about?
>>
>> The original intent was to revert back to the original peephole pattern
>> (pre-PR 46975) when the CC reg is still live, but that doesn't properly
>> maintain the CC state either (it just happened to pass in the test
>> case I was looking at because I only cared about the Z flag, which is
>> maintained the same).
>>
>> OK with the above bit left out?
>>
>
>

Sorry for the delay, I've been sidetracked onto other things.

Having looked at this patch I realized that we were missing a trick on 
ARMv5 and later, when a more efficient sequence exists, particularly for 
Cortex-A15.  By using CLZ we can avoid the need to set the condition 
code register at all, which gives us far more scheduling freedom.  It's 
also best not to unnecessarily clobber the condition code register even 
if there are other instructions in the sequence that do set/use the 
flags (the peepholer pass right at the end will do this optimization 
when it is useful), so I've tweaked some of the existing alternatives as 
well.

Finally, we can use peep2_regno_dead_p (rather than peep2_reg_dead_p) to 
avoid having to create extra match_operand values.

The result is that I've now committed the patch below.

R.

2013-06-19  Richard Earnshaw  <rearnsha@arm.com>

	arm.md (split for eq(reg, 0)): Add variants for ARMv5 and
	Thumb2.
	(peepholes for eq(reg, not-0)): Ensure condition register is
	dead after pattern.  Use more efficient sequences on ARMv5 and
	Thumb2.

[-- Attachment #2: gcc-eq.patch --]
[-- Type: text/plain, Size: 4475 bytes --]

--- gcc/config/arm/arm.md	(revision 200187)
+++ gcc/config/arm/arm.md	(local)
@@ -10021,6 +10021,16 @@ (define_split
 	(eq:SI (match_operand:SI 1 "s_register_operand" "")
 	       (const_int 0)))
    (clobber (reg:CC CC_REGNUM))]
+  "arm_arch5 && TARGET_32BIT"
+  [(set (match_dup 0) (clz:SI (match_dup 1)))
+   (set (match_dup 0) (lshiftrt:SI (match_dup 0) (const_int 5)))]
+)
+
+(define_split
+  [(set (match_operand:SI 0 "s_register_operand" "")
+	(eq:SI (match_operand:SI 1 "s_register_operand" "")
+	       (const_int 0)))
+   (clobber (reg:CC CC_REGNUM))]
   "TARGET_32BIT && reload_completed"
   [(parallel
     [(set (reg:CC CC_REGNUM)
@@ -10090,29 +10100,87 @@ (define_insn_and_split "*compare_scc"
 
 ;; Attempt to improve the sequence generated by the compare_scc splitters
 ;; not to use conditional execution.
+
+;; Rd = (eq (reg1) (const_int0))  // ARMv5
+;;	clz Rd, reg1
+;;	lsr Rd, Rd, #5
 (define_peephole2
   [(set (reg:CC CC_REGNUM)
 	(compare:CC (match_operand:SI 1 "register_operand" "")
-		    (match_operand:SI 2 "arm_rhs_operand" "")))
+		    (const_int 0)))
+   (cond_exec (ne (reg:CC CC_REGNUM) (const_int 0))
+	      (set (match_operand:SI 0 "register_operand" "") (const_int 0)))
+   (cond_exec (eq (reg:CC CC_REGNUM) (const_int 0))
+	      (set (match_dup 0) (const_int 1)))]
+  "arm_arch5 && TARGET_32BIT && peep2_regno_dead_p (3, CC_REGNUM)"
+  [(set (match_dup 0) (clz:SI (match_dup 1)))
+   (set (match_dup 0) (lshiftrt:SI (match_dup 0) (const_int 5)))]
+)
+
+;; Rd = (eq (reg1) (const_int0))  // !ARMv5
+;;	negs Rd, reg1
+;;	adc  Rd, Rd, reg1
+(define_peephole2
+  [(set (reg:CC CC_REGNUM)
+	(compare:CC (match_operand:SI 1 "register_operand" "")
+		    (const_int 0)))
    (cond_exec (ne (reg:CC CC_REGNUM) (const_int 0))
 	      (set (match_operand:SI 0 "register_operand" "") (const_int 0)))
    (cond_exec (eq (reg:CC CC_REGNUM) (const_int 0))
 	      (set (match_dup 0) (const_int 1)))
-   (match_scratch:SI 3 "r")]
-  "TARGET_32BIT"
+   (match_scratch:SI 2 "r")]
+  "TARGET_32BIT && peep2_regno_dead_p (3, CC_REGNUM)"
   [(parallel
     [(set (reg:CC CC_REGNUM)
-	  (compare:CC (match_dup 1) (match_dup 2)))
-     (set (match_dup 3) (minus:SI (match_dup 1) (match_dup 2)))])
+	  (compare:CC (const_int 0) (match_dup 1)))
+     (set (match_dup 2) (minus:SI (const_int 0) (match_dup 1)))])
+   (set (match_dup 0)
+	(plus:SI (plus:SI (match_dup 1) (match_dup 2))
+		 (geu:SI (reg:CC CC_REGNUM) (const_int 0))))]
+)
+
+;; Rd = (eq (reg1) (reg2/imm))	// ARMv5
+;;	sub  Rd, Reg1, reg2
+;;	clz  Rd, Rd
+;;	lsr  Rd, Rd, #5
+(define_peephole2
+  [(set (reg:CC CC_REGNUM)
+	(compare:CC (match_operand:SI 1 "register_operand" "")
+		    (match_operand:SI 2 "arm_rhs_operand" "")))
+   (cond_exec (ne (reg:CC CC_REGNUM) (const_int 0))
+	      (set (match_operand:SI 0 "register_operand" "") (const_int 0)))
+   (cond_exec (eq (reg:CC CC_REGNUM) (const_int 0))
+	      (set (match_dup 0) (const_int 1)))]
+  "arm_arch5 && TARGET_32BIT && peep2_regno_dead_p (3, CC_REGNUM)"
+  [(set (match_dup 0) (minus:SI (match_dup 1) (match_dup 2)))
+   (set (match_dup 0) (clz:SI (match_dup 0)))
+   (set (match_dup 0) (lshiftrt:SI (match_dup 0) (const_int 5)))]
+)
+
+
+;; Rd = (eq (reg1) (reg2/imm))	// ! ARMv5
+;;	sub  T1, Reg1, reg2
+;;	negs Rd, T1
+;;	adc  Rd, Rd, T1
+(define_peephole2
+  [(set (reg:CC CC_REGNUM)
+	(compare:CC (match_operand:SI 1 "register_operand" "")
+		    (match_operand:SI 2 "arm_rhs_operand" "")))
+   (cond_exec (ne (reg:CC CC_REGNUM) (const_int 0))
+	      (set (match_operand:SI 0 "register_operand" "") (const_int 0)))
+   (cond_exec (eq (reg:CC CC_REGNUM) (const_int 0))
+	      (set (match_dup 0) (const_int 1)))
+   (match_scratch:SI 3 "r")]
+  "TARGET_32BIT && peep2_regno_dead_p (3, CC_REGNUM)"
+  [(set (match_dup 3) (minus:SI (match_dup 1) (match_dup 2)))
    (parallel
     [(set (reg:CC CC_REGNUM)
 	  (compare:CC (const_int 0) (match_dup 3)))
      (set (match_dup 0) (minus:SI (const_int 0) (match_dup 3)))])
-   (parallel
-    [(set (match_dup 0)
-	  (plus:SI (plus:SI (match_dup 0) (match_dup 3))
-		   (geu:SI (reg:CC CC_REGNUM) (const_int 0))))
-     (clobber (reg:CC CC_REGNUM))])])
+   (set (match_dup 0)
+	(plus:SI (plus:SI (match_dup 0) (match_dup 3))
+		 (geu:SI (reg:CC CC_REGNUM) (const_int 0))))]
+)
 
 (define_insn "*cond_move"
   [(set (match_operand:SI 0 "s_register_operand" "=r,r,r")