This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][AArch64] Use CC_Z and CC_NZ with csinc and similar instructions
- From: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- To: Richard Henderson <rth at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: Richard Earnshaw <Richard dot Earnshaw at arm dot com>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>
- Date: Tue, 02 Sep 2014 16:34:06 +0100
- Subject: Re: [PATCH][AArch64] Use CC_Z and CC_NZ with csinc and similar instructions
- Authentication-results: sourceware.org; auth=none
- References: <53F1F068 dot 4070708 at arm dot com> <53F24B7D dot 1010200 at redhat dot com> <53F35128 dot 9010801 at arm dot com> <53F36C67 dot 4080803 at redhat dot com> <53F376D2 dot 8080505 at redhat dot com>
Hi Richard,
Sorry for the delay.
On 19/08/14 17:09, Richard Henderson wrote:
(define_special_predicate "cc_register_zero"
(match_code "reg")
{
return (REGNO (op) == CC_REGNUM
&& (GET_MODE (op) == CCmode
|| GET_MODE (op) == CC_Zmode
|| GET_MODE (op) == CC_NZmode));
})
... and now that I read the backend more closely, I see "_zero" was a bad name.
But more importantly, I see no connection between the comparison used and the
CCmode being accepted. And if we fix that, why are you restricting to just Z
and NZ? What's wrong with e.g. CFPmode?
I'm not sure why restricted the modes for csinc.
In the i386 backend, we check comparison+mode correspondence like
(match_operator 4 "ix86_carry_flag_operator"
[(match_operand 3 "flags_reg_operand") (const_int 0)])
I think you'll want something similar. In the case of CSINC, we can accept all
conditions, so let's start with the most general:
(match_operator:GPI 2 "aarch64_comparison_operation"
[(reg CC_REGNUM) (const_int 0)]
or even
(match_operand:GPI 2 "aarch64_comparison_operation" "")
with
(define_predicate "aarch64_comparison_operation"
(match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,"
"unordered,ordered,unlt,unle,unge,ungt")
{
if (XEXP (op, 1) != const0_rtx)
return false;
rtx op0 = XEXP (op, 0);
if (!REG_P (op0) || REGNO (op0) != CC_REGNUM)
return false;
return aarch64_get_condition_code (op) >= 0;
})
where aarch64_get_condition_code is
(1) exported
(2) adjusted to return "int" not "unsigned"
(3) adjusted to not abort, but return -1 for invalid combinations.
and the two existing users of aarch64_get_condition_code are adjusted to
gcc_assert that the return value is valid.
Implementing that seems to work fine. Bootstrap and testing were successful.
How's this version then?
Kyrill
2014-09-02 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
* config/aarch64/predicates.md (aarch64_comparison_operation):
New special predicate.
* config/aarch64/aarch64.md (*csinc2<mode>_insn): Use
aarch64_comparison_operation instead of matching an operator.
Update operand numbers.
(csinc3<mode>_insn): Likewise.
(*csinv3<mode>_insn): Likewise.
(*csneg3<mode>_insn): Likewise.
(ffs<mode>2): Update gen_csinc3<mode>_insn callsite.
* config/aarch64/aarch64.c (aarch64_get_condition_code): Export.
Return -1 instead of aborting on invalid condition codes.
(aarch64_print_operand): Update aarch64_get_condition_code callsites
to assert that the returned condition code is valid.
r~
commit a70dc696b967196d6662479a44682e3f423377ac
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date: Mon Aug 4 16:49:24 2014 +0100
[AArch64] Generalise condition code usage for csinc pattterns
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index b5335bf..d3be619 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -174,6 +174,7 @@ struct tune_params
};
HOST_WIDE_INT aarch64_initial_elimination_offset (unsigned, unsigned);
+int aarch64_get_condition_code (rtx);
bool aarch64_bitmask_imm (HOST_WIDE_INT val, enum machine_mode);
bool aarch64_cannot_change_mode_class (enum machine_mode,
enum machine_mode,
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index ba45d00..809d562 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3589,7 +3589,7 @@ aarch64_select_cc_mode (RTX_CODE code, rtx x, rtx y)
return CCmode;
}
-static unsigned
+int
aarch64_get_condition_code (rtx x)
{
enum machine_mode mode = GET_MODE (XEXP (x, 0));
@@ -3616,7 +3616,7 @@ aarch64_get_condition_code (rtx x)
case UNLE: return AARCH64_LE;
case UNGT: return AARCH64_HI;
case UNGE: return AARCH64_PL;
- default: gcc_unreachable ();
+ default: return -1;
}
break;
@@ -3633,7 +3633,7 @@ aarch64_get_condition_code (rtx x)
case GTU: return AARCH64_HI;
case LEU: return AARCH64_LS;
case LTU: return AARCH64_CC;
- default: gcc_unreachable ();
+ default: return -1;
}
break;
@@ -3652,7 +3652,7 @@ aarch64_get_condition_code (rtx x)
case GTU: return AARCH64_CC;
case LEU: return AARCH64_CS;
case LTU: return AARCH64_HI;
- default: gcc_unreachable ();
+ default: return -1;
}
break;
@@ -3663,7 +3663,7 @@ aarch64_get_condition_code (rtx x)
case EQ: return AARCH64_EQ;
case GE: return AARCH64_PL;
case LT: return AARCH64_MI;
- default: gcc_unreachable ();
+ default: return -1;
}
break;
@@ -3672,12 +3672,12 @@ aarch64_get_condition_code (rtx x)
{
case NE: return AARCH64_NE;
case EQ: return AARCH64_EQ;
- default: gcc_unreachable ();
+ default: return -1;
}
break;
default:
- gcc_unreachable ();
+ return -1;
break;
}
}
@@ -3795,39 +3795,48 @@ aarch64_print_operand (FILE *f, rtx x, char code)
break;
case 'm':
- /* Print a condition (eq, ne, etc). */
-
- /* CONST_TRUE_RTX means always -- that's the default. */
- if (x == const_true_rtx)
- return;
+ {
+ int cond_code;
+ /* Print a condition (eq, ne, etc). */
- if (!COMPARISON_P (x))
- {
- output_operand_lossage ("invalid operand for '%%%c'", code);
+ /* CONST_TRUE_RTX means always -- that's the default. */
+ if (x == const_true_rtx)
return;
- }
- fputs (aarch64_condition_codes[aarch64_get_condition_code (x)], f);
+ if (!COMPARISON_P (x))
+ {
+ output_operand_lossage ("invalid operand for '%%%c'", code);
+ return;
+ }
+
+ cond_code = aarch64_get_condition_code (x);
+ gcc_assert (cond_code >= 0);
+ fputs (aarch64_condition_codes[cond_code], f);
+ }
break;
case 'M':
- /* Print the inverse of a condition (eq <-> ne, etc). */
-
- /* CONST_TRUE_RTX means never -- that's the default. */
- if (x == const_true_rtx)
- {
- fputs ("nv", f);
- return;
- }
+ {
+ int cond_code;
+ /* Print the inverse of a condition (eq <-> ne, etc). */
- if (!COMPARISON_P (x))
- {
- output_operand_lossage ("invalid operand for '%%%c'", code);
- return;
- }
+ /* CONST_TRUE_RTX means never -- that's the default. */
+ if (x == const_true_rtx)
+ {
+ fputs ("nv", f);
+ return;
+ }
- fputs (aarch64_condition_codes[AARCH64_INVERSE_CONDITION_CODE
- (aarch64_get_condition_code (x))], f);
+ if (!COMPARISON_P (x))
+ {
+ output_operand_lossage ("invalid operand for '%%%c'", code);
+ return;
+ }
+ cond_code = aarch64_get_condition_code (x);
+ gcc_assert (cond_code >= 0);
+ fputs (aarch64_condition_codes[AARCH64_INVERSE_CONDITION_CODE
+ (cond_code)], f);
+ }
break;
case 'b':
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 3c51fd3..2e1394d 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -2598,9 +2598,8 @@ (define_insn "aarch64_<crc_variant>"
(define_insn "*csinc2<mode>_insn"
[(set (match_operand:GPI 0 "register_operand" "=r")
- (plus:GPI (match_operator:GPI 2 "aarch64_comparison_operator"
- [(match_operand:CC 3 "cc_register" "") (const_int 0)])
- (match_operand:GPI 1 "register_operand" "r")))]
+ (plus:GPI (match_operand 2 "aarch64_comparison_operation" "")
+ (match_operand:GPI 1 "register_operand" "r")))]
""
"csinc\\t%<w>0, %<w>1, %<w>1, %M2"
[(set_attr "type" "csel")]
@@ -2609,37 +2608,34 @@ (define_insn "*csinc2<mode>_insn"
(define_insn "csinc3<mode>_insn"
[(set (match_operand:GPI 0 "register_operand" "=r")
(if_then_else:GPI
- (match_operator:GPI 1 "aarch64_comparison_operator"
- [(match_operand:CC 2 "cc_register" "") (const_int 0)])
- (plus:GPI (match_operand:GPI 3 "register_operand" "r")
+ (match_operand 1 "aarch64_comparison_operation" "")
+ (plus:GPI (match_operand:GPI 2 "register_operand" "r")
(const_int 1))
- (match_operand:GPI 4 "aarch64_reg_or_zero" "rZ")))]
+ (match_operand:GPI 3 "aarch64_reg_or_zero" "rZ")))]
""
- "csinc\\t%<w>0, %<w>4, %<w>3, %M1"
+ "csinc\\t%<w>0, %<w>3, %<w>2, %M1"
[(set_attr "type" "csel")]
)
(define_insn "*csinv3<mode>_insn"
[(set (match_operand:GPI 0 "register_operand" "=r")
(if_then_else:GPI
- (match_operator:GPI 1 "aarch64_comparison_operator"
- [(match_operand:CC 2 "cc_register" "") (const_int 0)])
- (not:GPI (match_operand:GPI 3 "register_operand" "r"))
- (match_operand:GPI 4 "aarch64_reg_or_zero" "rZ")))]
+ (match_operand 1 "aarch64_comparison_operation" "")
+ (not:GPI (match_operand:GPI 2 "register_operand" "r"))
+ (match_operand:GPI 3 "aarch64_reg_or_zero" "rZ")))]
""
- "csinv\\t%<w>0, %<w>4, %<w>3, %M1"
+ "csinv\\t%<w>0, %<w>3, %<w>2, %M1"
[(set_attr "type" "csel")]
)
(define_insn "*csneg3<mode>_insn"
[(set (match_operand:GPI 0 "register_operand" "=r")
(if_then_else:GPI
- (match_operator:GPI 1 "aarch64_comparison_operator"
- [(match_operand:CC 2 "cc_register" "") (const_int 0)])
- (neg:GPI (match_operand:GPI 3 "register_operand" "r"))
- (match_operand:GPI 4 "aarch64_reg_or_zero" "rZ")))]
+ (match_operand 1 "aarch64_comparison_operation" "")
+ (neg:GPI (match_operand:GPI 2 "register_operand" "r"))
+ (match_operand:GPI 3 "aarch64_reg_or_zero" "rZ")))]
""
- "csneg\\t%<w>0, %<w>4, %<w>3, %M1"
+ "csneg\\t%<w>0, %<w>3, %<w>2, %M1"
[(set_attr "type" "csel")]
)
@@ -2896,7 +2892,7 @@ (define_expand "ffs<mode>2"
emit_insn (gen_rbit<mode>2 (operands[0], operands[1]));
emit_insn (gen_clz<mode>2 (operands[0], operands[0]));
- emit_insn (gen_csinc3<mode>_insn (operands[0], x, ccreg, operands[0], const0_rtx));
+ emit_insn (gen_csinc3<mode>_insn (operands[0], x, operands[0], const0_rtx));
DONE;
}
)
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
index 3dd83ca..c1510ca 100644
--- a/gcc/config/aarch64/predicates.md
+++ b/gcc/config/aarch64/predicates.md
@@ -202,6 +202,18 @@ (define_predicate "aarch64_reg_or_imm"
(define_special_predicate "aarch64_comparison_operator"
(match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,ordered,unlt,unle,unge,ungt"))
+(define_special_predicate "aarch64_comparison_operation"
+ (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,ordered,unlt,unle,unge,ungt")
+{
+ if (XEXP (op, 1) != const0_rtx)
+ return false;
+ rtx op0 = XEXP (op, 0);
+ if (!REG_P (op0) || REGNO (op0) != CC_REGNUM)
+ return false;
+ return aarch64_get_condition_code (op) >= 0;
+})
+
+
;; True if the operand is memory reference suitable for a load/store exclusive.
(define_predicate "aarch64_sync_memory_operand"
(and (match_operand 0 "memory_operand")