Patchwork [ARM] Fix unrecognizable vector comparisons

login
register
mail settings
Submitter Zhenqiang Chen
Date June 18, 2013, 8:50 a.m.
Message ID <CACgzC7B3jDU3jo1LRYzvQsTkn7hxRYs=LrOjMg5m_f+wEnOqCw@mail.gmail.com>
Download mbox | patch
Permalink /patch/252175/
State New
Headers show

Comments

Zhenqiang Chen - June 18, 2013, 8:50 a.m.
Hi,

During expand, function vcond<mode><mode> inverses some CMP, e.g.

   a LE b -> b GE a

But if "b" is "CONST0_RTX", "b GE a" will be an illegal insn.

(insn 933 932 934 113 (set (reg:V4SI 1027)
        (unspec:V4SI [
                (const_vector:V4SI [
                        (const_int 0 [0])
                        (const_int 0 [0])
                        (const_int 0 [0])
                        (const_int 0 [0])
                    ])
                (reg:V4SI 1023 [ vect_var_.49 ])
                (const_int 1 [0x1])
            ] UNSPEC_VCGE)) PUGHSlab/Mapping.c:567 -1
     (nil))

Refer https://bugs.launchpad.net/linaro-toolchain-binaries/+bug/1189445
for more. And the bug also happens for FSF trunk.

The similar issue
(https://bugs.launchpad.net/linaro-toolchain-binaries/+bug/1163942)
had fixed on AARCH64:
http://gcc.gnu.org/ml/gcc-patches/2013-04/msg00581.html

The patch is similar to the fix for aarch64.

Bootstrap and no make check regression on Panda Board.

Is it OK for trunk and 4.8?

Thanks!
-Zhenqiang

ChangeLog:

2013-06-18  Zhenqiang Chen  <zhenqiang.chen@linaro.org>

	* config/arm/neon.md (vcond): Fix floating-point vector
	comparisons against 0.
Ramana Radhakrishnan - June 18, 2013, 9:41 a.m.
On 06/18/13 09:50, Zhenqiang Chen wrote:
> Hi,
>
> During expand, function vcond<mode><mode> inverses some CMP, e.g.
>
>     a LE b -> b GE a
>
> But if "b" is "CONST0_RTX", "b GE a" will be an illegal insn.
>
> (insn 933 932 934 113 (set (reg:V4SI 1027)
>          (unspec:V4SI [
>                  (const_vector:V4SI [
>                          (const_int 0 [0])
>                          (const_int 0 [0])
>                          (const_int 0 [0])
>                          (const_int 0 [0])
>                      ])
>                  (reg:V4SI 1023 [ vect_var_.49 ])
>                  (const_int 1 [0x1])
>              ] UNSPEC_VCGE)) PUGHSlab/Mapping.c:567 -1
>       (nil))
>
> Refer https://bugs.launchpad.net/linaro-toolchain-binaries/+bug/1189445
> for more. And the bug also happens for FSF trunk.
>
> The similar issue
> (https://bugs.launchpad.net/linaro-toolchain-binaries/+bug/1163942)
> had fixed on AARCH64:
> http://gcc.gnu.org/ml/gcc-patches/2013-04/msg00581.html
>
> The patch is similar to the fix for aarch64.
>
> Bootstrap and no make check regression on Panda Board.
>
> Is it OK for trunk and 4.8?

No, not without an appropriate set of testcases that exercise these cases.


regards
Ramana

>
> Thanks!
> -Zhenqiang
>
> ChangeLog:
>
> 2013-06-18  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>
> 	* config/arm/neon.md (vcond): Fix floating-point vector
> 	comparisons against 0.
>
> diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
> index e814df0..9299ae5 100644
> --- a/gcc/config/arm/neon.md
> +++ b/gcc/config/arm/neon.md
> @@ -1671,6 +1671,7 @@
>   			     ? 3 : 1;
>     rtx magic_rtx = GEN_INT (magic_word);
>     int inverse = 0;
> +  int use_zero_form = 0;
>     int swap_bsl_operands = 0;
>     rtx mask = gen_reg_rtx (<V_cmp_result>mode);
>     rtx tmp = gen_reg_rtx (<V_cmp_result>mode);
> @@ -1681,12 +1682,16 @@
>     switch (GET_CODE (operands[3]))
>       {
>       case GE:
> +    case GT:
>       case LE:
> +    case LT:
>       case EQ:
> -      if (!REG_P (operands[5])
> -	  && (operands[5] != CONST0_RTX (<MODE>mode)))
> -	operands[5] = force_reg (<MODE>mode, operands[5]);
> -      break;
> +      if (operands[5] == CONST0_RTX (<MODE>mode))
> +	{
> +	  use_zero_form = 1;
> +	  break;
> +	}
> +      /* Fall through.  */
>       default:
>         if (!REG_P (operands[5]))
>   	operands[5] = force_reg (<MODE>mode, operands[5]);
> @@ -1737,7 +1742,26 @@
>   	 a GT b -> a GT b
>   	 a LE b -> b GE a
>   	 a LT b -> b GT a
> -	 a EQ b -> a EQ b  */
> +	 a EQ b -> a EQ b
> +	 Note that there also exist direct comparison against 0 forms,
> +	 so catch those as a special case.  */
> +      if (use_zero_form)
> +	{
> +	  inverse = 0;
> +	  switch (GET_CODE (operands[3]))
> +	    {
> +	    case LT:
> +	      base_comparison = gen_neon_vclt<mode>;
> +	      break;
> +	    case LE:
> +	      base_comparison = gen_neon_vcle<mode>;
> +	      break;
> +	    default:
> +	      /* Do nothing, other zero form cases already have the correct
> +		 base_comparison.  */
> +	      break;
> +	    }
> +	}
>
>         if (!inverse)
>   	emit_insn (base_comparison (mask, operands[4], operands[5], magic_rtx));
>

Patch

diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
index e814df0..9299ae5 100644
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -1671,6 +1671,7 @@ 
 			     ? 3 : 1;
   rtx magic_rtx = GEN_INT (magic_word);
   int inverse = 0;
+  int use_zero_form = 0;
   int swap_bsl_operands = 0;
   rtx mask = gen_reg_rtx (<V_cmp_result>mode);
   rtx tmp = gen_reg_rtx (<V_cmp_result>mode);
@@ -1681,12 +1682,16 @@ 
   switch (GET_CODE (operands[3]))
     {
     case GE:
+    case GT:
     case LE:
+    case LT:
     case EQ:
-      if (!REG_P (operands[5])
-	  && (operands[5] != CONST0_RTX (<MODE>mode)))
-	operands[5] = force_reg (<MODE>mode, operands[5]);
-      break;
+      if (operands[5] == CONST0_RTX (<MODE>mode))
+	{
+	  use_zero_form = 1;
+	  break;
+	}
+      /* Fall through.  */
     default:
       if (!REG_P (operands[5]))
 	operands[5] = force_reg (<MODE>mode, operands[5]);
@@ -1737,7 +1742,26 @@ 
 	 a GT b -> a GT b
 	 a LE b -> b GE a
 	 a LT b -> b GT a
-	 a EQ b -> a EQ b  */
+	 a EQ b -> a EQ b
+	 Note that there also exist direct comparison against 0 forms,
+	 so catch those as a special case.  */
+      if (use_zero_form)
+	{
+	  inverse = 0;
+	  switch (GET_CODE (operands[3]))
+	    {
+	    case LT:
+	      base_comparison = gen_neon_vclt<mode>;
+	      break;
+	    case LE:
+	      base_comparison = gen_neon_vcle<mode>;
+	      break;
+	    default:
+	      /* Do nothing, other zero form cases already have the correct
+		 base_comparison.  */
+	      break;
+	    }
+	}

       if (!inverse)
 	emit_insn (base_comparison (mask, operands[4], operands[5], magic_rtx));