diff mbox

[AArch64] Fix possible wrong code generation when comparing DImode values.

Message ID 1369297056-11890-1-git-send-email-james.greenhalgh@arm.com
State New
Headers show

Commit Message

James Greenhalgh May 23, 2013, 8:17 a.m. UTC 
Hi,

With the aarch64_cm<optab>di patterns a bug was introduced. While the
unsplit versions of these patterns, which operate in the
SIMD register set, do not clobber CC_REGNUM, the split versions, which
operate in the general purpose register set, do clobber CC_REGNUM.

This causes a problem if scheduling rearranges the unsplit version
of these instructions. For example, if we have:

  aarch64_cmeqdi_unsplit
  set_cc_flags
  jump

Then we could schedule as

  set_cc_flags
  aarch64_cmeqdi_unsplit
  jump

Because the unsplit version does not clobber CC_REGNUM.

If we now decide to split we get:

  set_cc_flags
  aarch64_cmeqdi_set_cc_flags
  aarch64_cmeqdi_use_cc_flags
  jump

And the jump uses the wrong value for cc_flags.

We fix this problem by adding the clobber of CC_REGNUM to
the aarch64_cm<optab>di patterns. This may restrict the
scheduling opportunities available, but should prevent
incorrect code generation.

Tested on aarch64-none-linux-gnu, aarch64-none-elf with no
regressions. The bug manifest itself in the libstdc++ testsuite,
so I've double checked there to ensure that the bug has cleared.

Thanks,
James

---
gcc/

2013-05-17  James Greenhalgh  <james.greenhalgh@arm.com>

	* config/aarch64/aarch64-simd.md
	(aarch64_cm<optab>di): Add clobber of CC_REGNUM to unsplit pattern.

Comments

Richard Earnshaw May 23, 2013, 9:36 a.m. UTC | #1 
On 23/05/13 09:17, James Greenhalgh wrote:
>
> Hi,
>
> With the aarch64_cm<optab>di patterns a bug was introduced. While the
> unsplit versions of these patterns, which operate in the
> SIMD register set, do not clobber CC_REGNUM, the split versions, which
> operate in the general purpose register set, do clobber CC_REGNUM.
>
> This causes a problem if scheduling rearranges the unsplit version
> of these instructions. For example, if we have:
>
>    aarch64_cmeqdi_unsplit
>    set_cc_flags
>    jump
>
> Then we could schedule as
>
>    set_cc_flags
>    aarch64_cmeqdi_unsplit
>    jump
>
> Because the unsplit version does not clobber CC_REGNUM.
>
> If we now decide to split we get:
>
>    set_cc_flags
>    aarch64_cmeqdi_set_cc_flags
>    aarch64_cmeqdi_use_cc_flags
>    jump
>
> And the jump uses the wrong value for cc_flags.
>
> We fix this problem by adding the clobber of CC_REGNUM to
> the aarch64_cm<optab>di patterns. This may restrict the
> scheduling opportunities available, but should prevent
> incorrect code generation.
>
> Tested on aarch64-none-linux-gnu, aarch64-none-elf with no
> regressions. The bug manifest itself in the libstdc++ testsuite,
> so I've double checked there to ensure that the bug has cleared.
>
> Thanks,
> James
>
> ---
> gcc/
>
> 2013-05-17  James Greenhalgh  <james.greenhalgh@arm.com>
>
> 	* config/aarch64/aarch64-simd.md
> 	(aarch64_cm<optab>di): Add clobber of CC_REGNUM to unsplit pattern.
>
>

OK.

R.
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 9069a73..f91cf81 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -3280,7 +3280,8 @@ 
 	  (COMPARISONS:DI
 	    (match_operand:DI 1 "register_operand" "w,w,r")
 	    (match_operand:DI 2 "aarch64_simd_reg_or_zero" "w,ZDz,r")
-	  )))]
+	  )))
+     (clobber (reg:CC CC_REGNUM))]
   "TARGET_SIMD"
   "@
   cm<n_optab>\t%d0, %d<cmp_1>, %d<cmp_2>
@@ -3291,15 +3292,7 @@ 
       happening in the 'w' constraint cases.  */
    && GP_REGNUM_P (REGNO (operands[0]))
    && GP_REGNUM_P (REGNO (operands[1]))"
-  [(set (reg:CC CC_REGNUM)
-    (compare:CC
-      (match_dup 1)
-      (match_dup 2)))
-  (set (match_dup 0)
-    (neg:DI
-      (COMPARISONS:DI
-	(match_operand 3 "cc_register" "")
-	(const_int 0))))]
+  [(const_int 0)]
   {
     enum machine_mode mode = SELECT_CC_MODE (<CMP>, operands[1], operands[2]);
     rtx cc_reg = aarch64_gen_compare_reg (<CMP>, operands[1], operands[2]);
@@ -3332,7 +3325,8 @@ 
 	  (UCOMPARISONS:DI
 	    (match_operand:DI 1 "register_operand" "w,r")
 	    (match_operand:DI 2 "aarch64_simd_reg_or_zero" "w,r")
-	  )))]
+	  )))
+    (clobber (reg:CC CC_REGNUM))]
   "TARGET_SIMD"
   "@
   cm<n_optab>\t%d0, %d<cmp_1>, %d<cmp_2>
@@ -3342,17 +3336,9 @@ 
       happening in the 'w' constraint cases.  */
    && GP_REGNUM_P (REGNO (operands[0]))
    && GP_REGNUM_P (REGNO (operands[1]))"
-  [(set (reg:CC CC_REGNUM)
-    (compare:CC
-      (match_dup 1)
-      (match_dup 2)))
-  (set (match_dup 0)
-    (neg:DI
-      (UCOMPARISONS:DI
-	(match_operand 3 "cc_register" "")
-	(const_int 0))))]
+  [(const_int 0)]
   {
-    enum machine_mode mode = SELECT_CC_MODE (<CMP>, operands[1], operands[2]);
+    enum machine_mode mode = CCmode;
     rtx cc_reg = aarch64_gen_compare_reg (<CMP>, operands[1], operands[2]);
     rtx comparison = gen_rtx_<CMP> (mode, operands[1], operands[2]);
     emit_insn (gen_cstoredi_neg (operands[0], comparison, cc_reg));
@@ -3385,7 +3371,8 @@ 
 	    (and:DI
 	      (match_operand:DI 1 "register_operand" "w,r")
 	      (match_operand:DI 2 "register_operand" "w,r"))
-	    (const_int 0))))]
+	    (const_int 0))))
+    (clobber (reg:CC CC_REGNUM))]
   "TARGET_SIMD"
   "@
   cmtst\t%d0, %d1, %d2
@@ -3395,16 +3382,7 @@ 
       happening in the 'w' constraint cases.  */
    && GP_REGNUM_P (REGNO (operands[0]))
    && GP_REGNUM_P (REGNO (operands[1]))"
-   [(set (reg:CC_NZ CC_REGNUM)
-	(compare:CC_NZ
-	 (and:DI (match_dup 1)
-		  (match_dup 2))
-	 (const_int 0)))
-  (set (match_dup 0)
-    (neg:DI
-      (ne:DI
-	(match_operand 3 "cc_register" "")
-	(const_int 0))))]
+  [(const_int 0)]
   {
     rtx and_tree = gen_rtx_AND (DImode, operands[1], operands[2]);
     enum machine_mode mode = SELECT_CC_MODE (NE, and_tree, const0_rtx);