This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [AArch64] Fix possible wrong code generation when comparing DImode values.
- From: James Greenhalgh <james dot greenhalgh at arm dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: marcus dot shawcroft at arm dot com, richard dot earnshaw at arm dot com
- Date: Mon, 24 Feb 2014 18:17:51 +0000
- Subject: Re: [AArch64] Fix possible wrong code generation when comparing DImode values.
- Authentication-results: sourceware.org; auth=none
- References: <519DE337 dot 9040902 at arm dot com>
Hi,
Back in May of last year I committed this patch to fix a nasty
bug where we would accidentally trash the CCmode registers.
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01337.html
Shorty thereafter, Richard Earnshaw contacted me with a suggestion
for a better fix. In short, in addition to splitting the general
purpose register case, we can also split the SIMD register case to
remove the CCmode clobber. This may save us from some redundant
comparisons.
I have no testcase, and only a gut-feeling that this looks like
the right thing to do, but the patch regresses clean on
aarch64-none-elf and aarch64-none-linux-gnu, so I believe it is
at least functionally correct.
OK for stage-1?
Thanks,
James
---
gcc/
2014-02-24 James Greenhalgh <james.greenhalgh@arm.com>
* config/aarch64/aarch64-simd.md
(aarch64_cm<optab>di): Always split.
(*aarch64_cm<optab>di): New.
(aarch64_cmtstdi): Always split.
(*aarch64_cmtstdi): New.
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 4dffb59e856aeaafb79007255d3b91a73ef1ef13..75a27443c246d4e56365b14dad559e99c499681c 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -3390,26 +3390,46 @@ (define_insn_and_split "aarch64_cm<optab
)))
(clobber (reg:CC CC_REGNUM))]
"TARGET_SIMD"
- "@
- cm<n_optab>\t%d0, %d<cmp_1>, %d<cmp_2>
- cm<optab>\t%d0, %d1, #0
- #"
- "reload_completed
- /* We need to prevent the split from
- happening in the 'w' constraint cases. */
- && GP_REGNUM_P (REGNO (operands[0]))
- && GP_REGNUM_P (REGNO (operands[1]))"
- [(const_int 0)]
+ "#"
+ "reload_completed"
+ [(set (match_operand:DI 0 "register_operand")
+ (neg:DI
+ (COMPARISONS:DI
+ (match_operand:DI 1 "register_operand")
+ (match_operand:DI 2 "aarch64_simd_reg_or_zero")
+ )))]
{
- enum machine_mode mode = SELECT_CC_MODE (<CMP>, operands[1], operands[2]);
- 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));
- DONE;
+ /* If we are in the general purpose register file,
+ we split to a sequence of comparison and store. */
+ if (GP_REGNUM_P (REGNO (operands[0]))
+ && GP_REGNUM_P (REGNO (operands[1])))
+ {
+ enum machine_mode mode = SELECT_CC_MODE (<CMP>, operands[1], operands[2]);
+ 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));
+ DONE;
+ }
+ /* Otherwise, we expand to a similar pattern which does not
+ clobber CC_REGNUM. */
}
[(set_attr "type" "neon_compare, neon_compare_zero, multiple")]
)
+(define_insn "*aarch64_cm<optab>di"
+ [(set (match_operand:DI 0 "register_operand" "=w,w")
+ (neg:DI
+ (COMPARISONS:DI
+ (match_operand:DI 1 "register_operand" "w,w")
+ (match_operand:DI 2 "aarch64_simd_reg_or_zero" "w,ZDz")
+ )))]
+ "TARGET_SIMD && reload_completed"
+ "@
+ cm<n_optab>\t%d0, %d<cmp_1>, %d<cmp_2>
+ cm<optab>\t%d0, %d1, #0"
+ [(set_attr "type" "neon_compare, neon_compare_zero")]
+)
+
;; cm(hs|hi)
(define_insn "aarch64_cm<optab><mode>"
@@ -3433,23 +3453,42 @@ (define_insn_and_split "aarch64_cm<optab
)))
(clobber (reg:CC CC_REGNUM))]
"TARGET_SIMD"
- "@
- cm<n_optab>\t%d0, %d<cmp_1>, %d<cmp_2>
- #"
- "reload_completed
- /* We need to prevent the split from
- happening in the 'w' constraint cases. */
- && GP_REGNUM_P (REGNO (operands[0]))
- && GP_REGNUM_P (REGNO (operands[1]))"
- [(const_int 0)]
+ "#"
+ "reload_completed"
+ [(set (match_operand:DI 0 "register_operand")
+ (neg:DI
+ (UCOMPARISONS:DI
+ (match_operand:DI 1 "register_operand")
+ (match_operand:DI 2 "aarch64_simd_reg_or_zero")
+ )))]
{
- 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));
- DONE;
+ /* If we are in the general purpose register file,
+ we split to a sequence of comparison and store. */
+ if (GP_REGNUM_P (REGNO (operands[0]))
+ && GP_REGNUM_P (REGNO (operands[1])))
+ {
+ 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));
+ DONE;
+ }
+ /* Otherwise, we expand to a similar pattern which does not
+ clobber CC_REGNUM. */
}
- [(set_attr "type" "neon_compare, neon_compare_zero")]
+ [(set_attr "type" "neon_compare,multiple")]
+)
+
+(define_insn "*aarch64_cm<optab>di"
+ [(set (match_operand:DI 0 "register_operand" "=w")
+ (neg:DI
+ (UCOMPARISONS:DI
+ (match_operand:DI 1 "register_operand" "w")
+ (match_operand:DI 2 "aarch64_simd_reg_or_zero" "w")
+ )))]
+ "TARGET_SIMD && reload_completed"
+ "cm<n_optab>\t%d0, %d<cmp_1>, %d<cmp_2>"
+ [(set_attr "type" "neon_compare")]
)
;; cmtst
@@ -3477,23 +3516,44 @@ (define_insn_and_split "aarch64_cmtstdi"
(const_int 0))))
(clobber (reg:CC CC_REGNUM))]
"TARGET_SIMD"
- "@
- cmtst\t%d0, %d1, %d2
- #"
- "reload_completed
- /* We need to prevent the split from
- happening in the 'w' constraint cases. */
- && GP_REGNUM_P (REGNO (operands[0]))
- && GP_REGNUM_P (REGNO (operands[1]))"
- [(const_int 0)]
+ "#"
+ "reload_completed"
+ [(set (match_operand:DI 0 "register_operand")
+ (neg:DI
+ (ne:DI
+ (and:DI
+ (match_operand:DI 1 "register_operand")
+ (match_operand:DI 2 "register_operand"))
+ (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);
- rtx cc_reg = aarch64_gen_compare_reg (NE, and_tree, const0_rtx);
- rtx comparison = gen_rtx_NE (mode, and_tree, const0_rtx);
- emit_insn (gen_cstoredi_neg (operands[0], comparison, cc_reg));
- DONE;
+ /* If we are in the general purpose register file,
+ we split to a sequence of comparison and store. */
+ if (GP_REGNUM_P (REGNO (operands[0]))
+ && GP_REGNUM_P (REGNO (operands[1])))
+ {
+ rtx and_tree = gen_rtx_AND (DImode, operands[1], operands[2]);
+ enum machine_mode mode = SELECT_CC_MODE (NE, and_tree, const0_rtx);
+ rtx cc_reg = aarch64_gen_compare_reg (NE, and_tree, const0_rtx);
+ rtx comparison = gen_rtx_NE (mode, and_tree, const0_rtx);
+ emit_insn (gen_cstoredi_neg (operands[0], comparison, cc_reg));
+ DONE;
+ }
+ /* Otherwise, we expand to a similar pattern which does not
+ clobber CC_REGNUM. */
}
+ [(set_attr "type" "neon_tst,multiple")]
+)
+
+(define_insn "*aarch64_cmtstdi"
+ [(set (match_operand:DI 0 "register_operand" "=w")
+ (neg:DI
+ (ne:DI
+ (and:DI
+ (match_operand:DI 1 "register_operand" "w")
+ (match_operand:DI 2 "register_operand" "w"))
+ (const_int 0))))]
+ "TARGET_SIMD"
+ "cmtst\t%d0, %d1, %d2"
[(set_attr "type" "neon_tst")]
)