diff mbox

[ARM,PR,68223] arm_[su]min_cmp pattern fails

Message ID 563C4CB3.9050703@linaro.org
State New
Headers show

Commit Message

Michael Collison Nov. 6, 2015, 6:46 a.m. UTC 
The patterns arm_smin_cmp and arm_umin_cmp patterns fail if operand 0 
and operand 2 are equal and both are less than operand 1. The solution 
is to remove the two patterns.

2015-11-06  Michael Collison  <michael.collison@linaro.org
         Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org>

     PR target/68223
     * gcc/config/arm/arm.md (*arm_smin_cmp): Remove pattern.
     (*arm_umin_cmp): Likewise.
     * gcc.target/arm/mincmp.c: Remove testcase.

Comments

Ramana Radhakrishnan Nov. 6, 2015, 7:49 a.m. UTC | #1 
On 06/11/15 06:46, Michael Collison wrote:
> The patterns arm_smin_cmp and arm_umin_cmp patterns fail if operand 0 and operand 2 are equal and both are less than operand 1. The solution is to remove the two patterns.

Agreed. There is no way this can be made to work without additional branches and that means the original pattern is probably more efficient.

> 
> 2015-11-06  Michael Collison  <michael.collison@linaro.org
>         Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org>
> 
>     PR target/68223

       
Instead just say .. (watching out formatting in the Changelog).

Revert:
2015-08-01  Michael Collison  <michael.collison@linaro.org
            Ramana Radhakrishnan  <ramana.radhakrishnan@linaro.org>

        * config/arm/arm.md (*arm_smin_cmp): New pattern.
        (*arm_umin_cmp): Likewise

I would like to have a simple execution test added as a follow-up and not rely on a test failing in BE testing to show this failure up.

You do not need approval to revert your own patch unless you see that it's become enmeshed with other people's work - So, this is OK assuming no regressions.

regards
Ramana
diff mbox

Patch

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 02e147e..6ba1ec3 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -3455,44 +3455,6 @@ 
    (set_attr "type" "multiple,multiple")]
 )
 
-;; t = (s/u)min (x, y)
-;; cc = cmp (t, z)
-;; is the same as
-;; cmp x, z
-;; cmpge(u) y, z
-
-(define_insn_and_split "*arm_smin_cmp"
-  [(set (reg:CC CC_REGNUM)
-	(compare:CC
-	 (smin:SI (match_operand:SI 0 "s_register_operand" "r")
-		  (match_operand:SI 1 "s_register_operand" "r"))
-	 (match_operand:SI 2 "s_register_operand" "r")))]
-  "TARGET_32BIT"
-  "#"
-  "&& reload_completed"
-  [(set (reg:CC CC_REGNUM)
-	(compare:CC (match_dup 0) (match_dup 2)))
-   (cond_exec (ge:CC (reg:CC CC_REGNUM) (const_int 0))
-	      (set (reg:CC CC_REGNUM)
-		   (compare:CC (match_dup 1) (match_dup 2))))]
-)
-
-(define_insn_and_split "*arm_umin_cmp"
-  [(set (reg:CC CC_REGNUM)
-	(compare:CC
-	 (umin:SI (match_operand:SI 0 "s_register_operand" "r")
-		  (match_operand:SI 1 "s_register_operand" "r"))
-	 (match_operand:SI 2 "s_register_operand" "r")))]
-  "TARGET_32BIT"
-  "#"
-  "&& reload_completed"
-  [(set (reg:CC CC_REGNUM)
-	(compare:CC (match_dup 0) (match_dup 2)))
-   (cond_exec (geu:CC (reg:CC CC_REGNUM) (const_int 0))
-	      (set (reg:CC CC_REGNUM)
-		   (compare:CC (match_dup 1) (match_dup 2))))]
-)
-
 (define_expand "umaxsi3"
   [(parallel [
     (set (match_operand:SI 0 "s_register_operand" "")
diff --git a/gcc/testsuite/gcc.target/arm/mincmp.c b/gcc/testsuite/gcc.target/arm/mincmp.c
deleted file mode 100644
index ade3bd9..0000000
--- a/gcc/testsuite/gcc.target/arm/mincmp.c
+++ /dev/null
@@ -1,20 +0,0 @@ 
-/* { dg-do compile } */
-/* { dg-options "-O2" } */
-/* { dg-require-effective-target arm32 } */
-
-#define min(x, y) ((x) <= (y)) ? (x) : (y)
-
-unsigned int 
-foo (unsigned int i, unsigned int x, unsigned int y)
-{
-  return i < (min (x, y));
-}
-
-int 
-bar (int i, int x, int y)
-{
-  return i < (min (x, y));
-}
-
-/* { dg-final { scan-assembler "cmpcs" } } */
-/* { dg-final { scan-assembler "cmpge" } } */
-- 
1.9.1