[RFA] Nonfunctioning split in rs6000 back-end

[RFA] Nonfunctioning split in rs6000 back-end

[Paolo Bonzini]

While researching who is really using flow's computed LOG_LINKS, I found 
a define_split in the rs6000 back-end that uses them through 
find_single_use.  It turns out the only users are combine, this split, 
and a function in regmove.

The split dates back to revision 1.5 of old-gcc.

;; If we are comparing a register for equality with a large constant,
;; we can do this with an XOR followed by a compare.  But we need a scratch
;; register for the result of the XOR.

(define_split
  [(set (match_operand:CC 0 "cc_reg_operand" "")
        (compare:CC (match_operand:SI 1 "gpc_reg_operand" "")
                    (match_operand:SI 2 "non_short_cint_operand" "")))
   (clobber (match_operand:SI 3 "gpc_reg_operand" ""))]
  "find_single_use (operands[0], insn, 0)
   && (GET_CODE (*find_single_use (operands[0], insn, 0)) == EQ
       || GET_CODE (*find_single_use (operands[0], insn, 0)) == NE)"
  [(set (match_dup 3) (xor:SI (match_dup 1) (match_dup 4)))
   (set (match_dup 0) (compare:CC (match_dup 3) (match_dup 5)))]

(etc.)

This split would turn

        lis r0,0x1234
        ori r0,r0,0x5678
        cmpwi cr7,r3,r0
        beq cr7,L6

into

        xoris r0,r3,0x1234
        cmpwi cr7,r0,0x5678
        beq cr7,L6

Nice trick, but it will not trigger, because the non_short_cint_operand 
will have been split already before this pattern is matched.  I see two 
possibilities:

1) turning it into a peephole2 like the following:

(define_peephole2
  [(set (match_operand:GPR 0 "register_operand")
        (match_operand:GPR 1 "logical_operand" ""))
   (set (match_dup 0) (match_operator:GPR 3 "boolean_or_operator"
                  [(match_dup 0)
                   (match_operand:GPR 2 "logical_operand" "")]))
   (set (match_operand:CC 4 "cc_reg_operand" "")
        (compare:CC (match_operand:GPR 5 "gpc_reg_operand" "")
                    (match_dup 0)))
   (set (pc)
        (if_then_else (match_operator 6 "equality_operator"
                  [(match_dup 4) (const_int 0)])
                 (match_operand 7 "" "")
                 (match_operand 8 "" "")))]
  "peep2_reg_dead_p (3, operands[0]) && printf (\"^&\")"
 [(set (match_dup 0) (xor:GPR (match_dup 5) (match_dup 9)))
  (set (match_dup 4) (compare:CC (match_dup 0) (match_dup 10)))
  (set (pc) (if_then_else (match_dup 6) (match_dup 7) (match_dup 8)))]

{
  /* Get the constant we are comparing against, and see what it looks like
     when sign-extended from 16 to 32 bits.  Then see what constant we could
     XOR with SEXTC to get the sign-extended value.  */
  rtx cnst = simplify_const_binary_operation (GET_CODE (operands[3]),
                                              GET_MODE (operands[3]),
                                              operands[1], operands[2]);
  HOST_WIDE_INT c = INTVAL (cnst);
  HOST_WIDE_INT sextc = ((c & 0xffff) ^ 0x8000) - 0x8000;
  HOST_WIDE_INT xorv = c ^ sextc;

  operands[9] = GEN_INT (xorv);
  operands[10] = GEN_INT (sextc);
})

I'm testing a patch that does this replacement, and I can post it 
tomorrow morning.  It has triggered only a dozen times so far (half in 
libgcc, half in the compiler), but it may be worth keeping it.  However, 
another possibility is...

2) ... entirely getting rid of it.

What do you suggest?

Paolo

Re: [RFA] Nonfunctioning split in rs6000 back-end

[David Edelsohn]

>>>>> Paolo Bonzini writes:

Paolo> I'm testing a patch that does this replacement, and I can post it 
Paolo> tomorrow morning.  It has triggered only a dozen times so far (half in 
Paolo> libgcc, half in the compiler), but it may be worth keeping it.

	It would be nice to keep this type of optimization if the
re-engineered version works.

Thanks, David

Re: [RFA] Nonfunctioning split in rs6000 back-end

[Paolo Bonzini]

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

David Edelsohn wrote:

>>>>>>Paolo Bonzini writes:
>>>>>>            
>>>>>>
>
>Paolo> I'm testing a patch that does this replacement, and I can post it 
>Paolo> tomorrow morning.  It has triggered only a dozen times so far (half in 
>Paolo> libgcc, half in the compiler), but it may be worth keeping it.
>
>	It would be nice to keep this type of optimization if the
>re-engineered version works.
>
>  
>
Here it is, bootstrapped and regtested on powerpc-apple-darwin8.1.0.

Ok for mainline?

Paolo

[-- Attachment #2: rs6000-peephole2.patch --]
[-- Type: text/plain, Size: 4035 bytes --]

2005-08-22  Paolo Bonzini  <bonzini@gnu.org>

	* config/rs6000/predicates.md (equality_operator): New.
	* config/rs6000/rs6000.md: Rewrite as a peephole2 the split for
	comparison with a large constant.

Index: config/rs6000/predicates.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/rs6000/predicates.md,v
retrieving revision 1.23
diff -p -u -r1.23 predicates.md
--- config/rs6000/predicates.md	11 Aug 2005 21:18:11 -0000	1.23
+++ config/rs6000/predicates.md	22 Aug 2005 20:44:32 -0000
@@ -710,6 +710,10 @@
 (define_predicate "boolean_or_operator"
   (match_code "ior,xor"))
 
+;; Return true if operand is an equality operator.
+(define_special_predicate "equality_operator"
+  (match_code "eq,ne"))
+
 ;; Return true if operand is MIN or MAX operator.
 (define_predicate "min_max_operator"
   (match_code "smin,smax,umin,umax"))
Index: rs6000.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/rs6000/rs6000.md,v
retrieving revision 1.400
diff -p -u -r1.400 rs6000.md
--- rs6000.md	20 Aug 2005 04:17:17 -0000	1.400
+++ rs6000.md	22 Aug 2005 20:41:44 -0000
@@ -10727,32 +10727,43 @@
   [(set_attr "type" "cmp")])
 
 ;; If we are comparing a register for equality with a large constant,
-;; we can do this with an XOR followed by a compare.  But we need a scratch
-;; register for the result of the XOR.
-
-(define_split
-  [(set (match_operand:CC 0 "cc_reg_operand" "")
-	(compare:CC (match_operand:SI 1 "gpc_reg_operand" "")
-		    (match_operand:SI 2 "non_short_cint_operand" "")))
-   (clobber (match_operand:SI 3 "gpc_reg_operand" ""))]
-  "find_single_use (operands[0], insn, 0)
-   && (GET_CODE (*find_single_use (operands[0], insn, 0)) == EQ
-       || GET_CODE (*find_single_use (operands[0], insn, 0)) == NE)"
-  [(set (match_dup 3) (xor:SI (match_dup 1) (match_dup 4)))
-   (set (match_dup 0) (compare:CC (match_dup 3) (match_dup 5)))]
-  "
-{
-  /* Get the constant we are comparing against, C,  and see what it looks like
-     sign-extended to 16 bits.  Then see what constant could be XOR'ed
-     with C to get the sign-extended value.  */
-
-  HOST_WIDE_INT c = INTVAL (operands[2]);
+;; we can do this with an XOR followed by a compare.  But this is profitable
+;; only if the large constant is only used for the comparison (and in this
+;; case we already have a register to reuse as scratch).
+
+(define_peephole2
+  [(set (match_operand:GPR 0 "register_operand")
+        (match_operand:GPR 1 "logical_operand" ""))
+   (set (match_dup 0) (match_operator:GPR 3 "boolean_or_operator"
+		       [(match_dup 0)
+			(match_operand:GPR 2 "logical_operand" "")]))
+   (set (match_operand:CC 4 "cc_reg_operand" "")
+        (compare:CC (match_operand:GPR 5 "gpc_reg_operand" "")
+                    (match_dup 0)))
+   (set (pc)
+        (if_then_else (match_operator 6 "equality_operator"
+                       [(match_dup 4) (const_int 0)])
+                      (match_operand 7 "" "")
+                      (match_operand 8 "" "")))]
+  "peep2_reg_dead_p (3, operands[0])"
+ [(set (match_dup 0) (xor:GPR (match_dup 5) (match_dup 9)))
+  (set (match_dup 4) (compare:CC (match_dup 0) (match_dup 10)))
+  (set (pc) (if_then_else (match_dup 6) (match_dup 7) (match_dup 8)))]
+ 
+{
+  /* Get the constant we are comparing against, and see what it looks like
+     when sign-extended from 16 to 32 bits.  Then see what constant we could
+     XOR with SEXTC to get the sign-extended value.  */
+  rtx cnst = simplify_const_binary_operation (GET_CODE (operands[3]),
+					      GET_MODE (operands[3]),
+					      operands[1], operands[2]);
+  HOST_WIDE_INT c = INTVAL (cnst);
   HOST_WIDE_INT sextc = ((c & 0xffff) ^ 0x8000) - 0x8000;
   HOST_WIDE_INT xorv = c ^ sextc;
 
-  operands[4] = GEN_INT (xorv);
-  operands[5] = GEN_INT (sextc);
-}")
+  operands[9] = GEN_INT (xorv);
+  operands[10] = GEN_INT (sextc);
+})
 
 (define_insn "*cmpsi_internal2"
   [(set (match_operand:CCUNS 0 "cc_reg_operand" "=y")

Re: [RFA] Nonfunctioning split in rs6000 back-end

[Giovanni Bajo]

Paolo Bonzini <paolo.bonzini@lu.unisi.ch> wrote:

> While researching who is really using flow's computed LOG_LINKS, I
> found 
> a define_split in the rs6000 back-end that uses them through
> find_single_use.  It turns out the only users are combine, this split,
> and a function in regmove.


See also:
http://gcc.gnu.org/ml/gcc-patches/2004-01/msg02371.html

Giovanni Bajo