Re: [ARM] Fix a performance regression from the fix for PR49030

[Richard Sandiford]

Ramana Radhakrishnan ramana.radhakrish...@linaro.org writes:
 On 20 September 2011 09:38, Richard Sandiford
 Otherwise, all expanders use expandable_comparison_operator instead.
 This restores the previous behaviour for them, and I went through each
 one to try to make sure that it was handled correctly.

 Tested on arm-linux-gnueabi.  OK to install?

 This is OK. Please add some tests for FPmode comparisons as well
  just so that we catch these early next time.

OK, thanks.  I also beefed up the integer tests to cover ?: and
conditional stores.  Here's what I installed after testing on
arm-linux-gnueabi.

Richard


gcc/
* config/arm/predicates.md (expandable_comparison_operator): New
predicate, extracted from...
(arm_comparison_operator): ...here.
* config/arm/arm.md (cbranchsi4, cbranchsf4, cbranchdf4, cbranchdi4)
(cstoresi4, cstoresf4, cstoredf4, cstoredi4, movsicc, movsfcc)
(movdfcc): Use expandable_comparison_operator.

gcc/testsuite/
* gcc.target/arm/cmp-1.c: New test.
* gcc.target/arm/cmp-2.c: Likewise.

Index: gcc/config/arm/predicates.md
===
--- gcc/config/arm/predicates.md2011-09-22 11:29:57.340345941 +0100
+++ gcc/config/arm/predicates.md2011-09-22 11:32:44.593058642 +0100
@@ -249,9 +249,14 @@ (define_special_predicate equality_oper
 
 ;; True for integer comparisons and, if FP is active, for comparisons
 ;; other than LTGT or UNEQ.
+(define_special_predicate expandable_comparison_operator
+  (match_code eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,
+  unordered,ordered,unlt,unle,unge,ungt))
+
+;; Likewise, but only accept comparisons that are directly supported
+;; by ARM condition codes.
 (define_special_predicate arm_comparison_operator
-  (and (match_code eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,
-   unordered,ordered,unlt,unle,unge,ungt)
+  (and (match_operand 0 expandable_comparison_operator)
(match_test maybe_get_arm_condition_code (op) != ARM_NV)))
 
 (define_special_predicate lt_ge_comparison_operator
Index: gcc/config/arm/arm.md
===
--- gcc/config/arm/arm.md   2011-09-22 11:29:57.339345943 +0100
+++ gcc/config/arm/arm.md   2011-09-22 11:32:44.592058643 +0100
@@ -6791,7 +6791,7 @@ (define_insn movmem8b
 
 (define_expand cbranchsi4
   [(set (pc) (if_then_else
- (match_operator 0 arm_comparison_operator
+ (match_operator 0 expandable_comparison_operator
   [(match_operand:SI 1 s_register_operand )
(match_operand:SI 2 nonmemory_operand )])
  (label_ref (match_operand 3  ))
@@ -6842,7 +6842,7 @@ (define_expand cbranchqi4
 
 (define_expand cbranchsf4
   [(set (pc) (if_then_else
- (match_operator 0 arm_comparison_operator
+ (match_operator 0 expandable_comparison_operator
   [(match_operand:SF 1 s_register_operand )
(match_operand:SF 2 arm_float_compare_operand )])
  (label_ref (match_operand 3  ))
@@ -6854,7 +6854,7 @@ (define_expand cbranchsf4
 
 (define_expand cbranchdf4
   [(set (pc) (if_then_else
- (match_operator 0 arm_comparison_operator
+ (match_operator 0 expandable_comparison_operator
   [(match_operand:DF 1 s_register_operand )
(match_operand:DF 2 arm_float_compare_operand )])
  (label_ref (match_operand 3  ))
@@ -6866,7 +6866,7 @@ (define_expand cbranchdf4
 
 (define_expand cbranchdi4
   [(set (pc) (if_then_else
- (match_operator 0 arm_comparison_operator
+ (match_operator 0 expandable_comparison_operator
   [(match_operand:DI 1 cmpdi_operand )
(match_operand:DI 2 cmpdi_operand )])
  (label_ref (match_operand 3  ))
@@ -7721,7 +7721,7 @@ (define_insn *mov_notscc
 
 (define_expand cstoresi4
   [(set (match_operand:SI 0 s_register_operand )
-   (match_operator:SI 1 arm_comparison_operator
+   (match_operator:SI 1 expandable_comparison_operator
 [(match_operand:SI 2 s_register_operand )
  (match_operand:SI 3 reg_or_int_operand )]))]
   TARGET_32BIT || TARGET_THUMB1
@@ -7857,7 +7857,7 @@ (define_expand cstoresi4
 
 (define_expand cstoresf4
   [(set (match_operand:SI 0 s_register_operand )
-   (match_operator:SI 1 arm_comparison_operator
+   (match_operator:SI 1 expandable_comparison_operator
 [(match_operand:SF 2 s_register_operand )
  (match_operand:SF 3 arm_float_compare_operand )]))]
   TARGET_32BIT  TARGET_HARD_FLOAT
@@ -7867,7 +7867,7 @@ (define_expand cstoresf4
 
 (define_expand cstoredf4
   [(set (match_operand:SI 0 s_register_operand )
-   (match_operator:SI 1 arm_comparison_operator
+   (match_operator:SI 1 expandable_comparison_operator
 [(match_operand:DF 2 s_register_operand )
  (match_operand:DF 3 

Re: [ARM] Fix a performance regression from the fix for PR49030

[Ramana Radhakrishnan]

On 20 September 2011 09:38, Richard Sandiford
richard.sandif...@linaro.org wrote:
 My fix for PR49030 had the unintended side-effect of forcing libcalls
 to be used for some DImode comparisons.  The problem (which I should
 have noticed at the time, sorry) is that arm_comparison_operator is
 used for both expanders and insns.  The patch fixed the definition
 for insns, but made it too tight for expanders.

 The expanders can't use the generic comparison_operator predicate
 because ARM has no support for UNEQ and LTGT.  So this patch introduces
 an expandable_comparison_operator that the expanders can use instead.

 I've left incscc and decscc using arm_comparison_operator, since they
 expand directly into instructions without any manipulation.  These
 expanders appear to be dead anyway, so I'd be happy to remove them if
 that seems better.  It should probably be a separate patch though.


OK as a followup patch.

 Otherwise, all expanders use expandable_comparison_operator instead.
 This restores the previous behaviour for them, and I went through each
 one to try to make sure that it was handled correctly.

 Tested on arm-linux-gnueabi.  OK to install?

This is OK. Please add some tests for FPmode comparisons as well
 just so that we catch these early next time.


cheers
Ramana