[gcc r10-6036] aarch64: Don't raise FE_INVALID for -__builtin_isgreater [PR93133]
Richard Sandiford
rsandifo@gcc.gnu.org
Fri Jan 17 12:18:00 GMT 2020
https://gcc.gnu.org/g:865257c447cc50f5819e9b53da83145f3c36c305
commit r10-6036-g865257c447cc50f5819e9b53da83145f3c36c305
Author: Richard Sandiford <richard.sandiford@arm.com>
Date: Thu Jan 16 19:22:20 2020 +0000
aarch64: Don't raise FE_INVALID for -__builtin_isgreater [PR93133]
AIUI, the main purpose of REVERSE_CONDITION is to take advantage of
any integer vs. FP information encoded in the CC mode, particularly
when handling LT, LE, GE and GT. For integer comparisons we can
safely map LT->GE, LE->GT, GE->LT and GT->LE, but for float comparisons
this would usually be invalid without -ffinite-math-only.
The aarch64 definition of REVERSE_CONDITION used
reverse_condition_maybe_unordered for FP comparisons, which had the
effect of converting an unordered-signalling LT, LE, GE or GT into a
quiet UNGE, UNGT, UNLT or UNLE. And it would do the same in reverse:
convert a quiet UN* into an unordered-signalling comparison.
This would be safe in practice (although a little misleading) if we
always used a compare:CCFP or compare:CCFPE to do the comparison and
then used (gt (reg:CCFP/CCFPE CC_REGNUM) (const_int 0)) etc. to test
the result. In that case any signal is raised by the compare and the
choice of quiet vs. signalling relations doesn't matter when testing
the result. The problem is that we also want to use GT directly on
float registers, where any signal is raised by the comparison operation
itself and so must follow the normal rtl rules (GT signalling,
UNLE quiet).
I think the safest fix is to make REVERSIBLE_CC_MODE return false
for FP comparisons. We can then use the default REVERSE_CONDITION
for integer comparisons and the usual conservatively-correct
reversed_comparison_code_parts behaviour for FP comparisons.
Unfortunately reversed_comparison_code_parts doesn't yet handle
-ffinite-math-only, but that's probably GCC 11 material.
A downside is that:
int f (float x, float y) { return !(x < y); }
now generates:
fcmpe s0, s1
cset w0, mi
eor w0, w0, 1
ret
without -ffinite-math-only. Maybe for GCC 11 we should define rtx
codes for all IEEE comparisons, so that we don't have this kind of
representational gap.
Changing REVERSE_CONDITION itself is pretty easy. However, the macro
was also used in the ccmp handling, which relied on being able to
reverse all comparisons. The patch adds new reversed patterns for
cases in which the original condition needs to be kept.
The test is based on gcc.dg/torture/pr91323.c. It might well fail
on other targets that have similar bugs; please XFAIL as appropriate
if you don't want to fix the target for GCC 10.
2020-01-17 Richard Sandiford <richard.sandiford@arm.com>
gcc/
* config/aarch64/aarch64.h (REVERSIBLE_CC_MODE): Return false
for FP modes.
(REVERSE_CONDITION): Delete.
* config/aarch64/iterators.md (CC_ONLY): New mode iterator.
(CCFP_CCFPE): Likewise.
(e): New mode attribute.
* config/aarch64/aarch64.md (ccmp<GPI:mode>): Rename to...
(@ccmp<CC_ONLY:mode><GPI:mode>): ...this, using CC_ONLY instead of CC.
(fccmp<GPF:mode>, fccmpe<GPF:mode>): Merge into...
(@ccmp<CCFP_CCFPE:mode><GPF:mode>): ...this combined pattern.
(@ccmp<CC_ONLY:mode><GPI:mode>_rev): New pattern.
(@ccmp<CCFP_CCFPE:mode><GPF:mode>_rev): Likewise.
* config/aarch64/aarch64.c (aarch64_gen_compare_reg): Update
name of generator from gen_ccmpdi to gen_ccmpccdi.
(aarch64_gen_ccmp_next): Use code_for_ccmp. If we want to reverse
the previous comparison but aren't able to, use the new ccmp_rev
patterns instead.
Diff:
---
gcc/ChangeLog | 20 +++++++++++
gcc/config/aarch64/aarch64.c | 30 ++++++++++------
gcc/config/aarch64/aarch64.h | 10 +++---
gcc/config/aarch64/aarch64.md | 64 ++++++++++++++++++++++++----------
gcc/config/aarch64/iterators.md | 6 ++++
gcc/testsuite/gcc.dg/torture/pr93133.c | 41 ++++++++++++++++++++++
6 files changed, 136 insertions(+), 35 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 423899d..6c6d586 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,25 @@
2020-01-17 Richard Sandiford <richard.sandiford@arm.com>
+ * config/aarch64/aarch64.h (REVERSIBLE_CC_MODE): Return false
+ for FP modes.
+ (REVERSE_CONDITION): Delete.
+ * config/aarch64/iterators.md (CC_ONLY): New mode iterator.
+ (CCFP_CCFPE): Likewise.
+ (e): New mode attribute.
+ * config/aarch64/aarch64.md (ccmp<GPI:mode>): Rename to...
+ (@ccmp<CC_ONLY:mode><GPI:mode>): ...this, using CC_ONLY instead of CC.
+ (fccmp<GPF:mode>, fccmpe<GPF:mode>): Merge into...
+ (@ccmp<CCFP_CCFPE:mode><GPF:mode>): ...this combined pattern.
+ (@ccmp<CC_ONLY:mode><GPI:mode>_rev): New pattern.
+ (@ccmp<CCFP_CCFPE:mode><GPF:mode>_rev): Likewise.
+ * config/aarch64/aarch64.c (aarch64_gen_compare_reg): Update
+ name of generator from gen_ccmpdi to gen_ccmpccdi.
+ (aarch64_gen_ccmp_next): Use code_for_ccmp. If we want to reverse
+ the previous comparison but aren't able to, use the new ccmp_rev
+ patterns instead.
+
+2020-01-17 Richard Sandiford <richard.sandiford@arm.com>
+
* gimplify.c (gimplify_return_expr): Use poly_int_tree_p rather
than testing directly for INTEGER_CST.
(gimplify_target_expr, gimplify_omp_depend): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 600a238..3660ce7 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2345,9 +2345,9 @@ aarch64_gen_compare_reg (RTX_CODE code, rtx x, rtx y)
rtx x_hi = operand_subword (x, 1, 0, TImode);
rtx y_hi = operand_subword (y, 1, 0, TImode);
- emit_insn (gen_ccmpdi (cc_reg, cc_reg, x_hi, y_hi,
- gen_rtx_EQ (cc_mode, cc_reg, const0_rtx),
- GEN_INT (AARCH64_EQ)));
+ emit_insn (gen_ccmpccdi (cc_reg, cc_reg, x_hi, y_hi,
+ gen_rtx_EQ (cc_mode, cc_reg, const0_rtx),
+ GEN_INT (AARCH64_EQ)));
}
else
{
@@ -20270,24 +20270,20 @@ aarch64_gen_ccmp_next (rtx_insn **prep_seq, rtx_insn **gen_seq, rtx prev,
case E_HImode:
case E_SImode:
cmp_mode = SImode;
- icode = CODE_FOR_ccmpsi;
break;
case E_DImode:
cmp_mode = DImode;
- icode = CODE_FOR_ccmpdi;
break;
case E_SFmode:
cmp_mode = SFmode;
cc_mode = aarch64_select_cc_mode ((rtx_code) cmp_code, op0, op1);
- icode = cc_mode == CCFPEmode ? CODE_FOR_fccmpesf : CODE_FOR_fccmpsf;
break;
case E_DFmode:
cmp_mode = DFmode;
cc_mode = aarch64_select_cc_mode ((rtx_code) cmp_code, op0, op1);
- icode = cc_mode == CCFPEmode ? CODE_FOR_fccmpedf : CODE_FOR_fccmpdf;
break;
default:
@@ -20295,6 +20291,8 @@ aarch64_gen_ccmp_next (rtx_insn **prep_seq, rtx_insn **gen_seq, rtx prev,
return NULL_RTX;
}
+ icode = code_for_ccmp (cc_mode, cmp_mode);
+
op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode, unsignedp);
op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode, unsignedp);
if (!op0 || !op1)
@@ -20310,9 +20308,21 @@ aarch64_gen_ccmp_next (rtx_insn **prep_seq, rtx_insn **gen_seq, rtx prev,
if (bit_code != AND)
{
- prev = gen_rtx_fmt_ee (REVERSE_CONDITION (GET_CODE (prev),
- GET_MODE (XEXP (prev, 0))),
- VOIDmode, XEXP (prev, 0), const0_rtx);
+ /* Treat the ccmp patterns as canonical and use them where possible,
+ but fall back to ccmp_rev patterns if there's no other option. */
+ rtx_code prev_code = GET_CODE (prev);
+ machine_mode prev_mode = GET_MODE (XEXP (prev, 0));
+ if ((prev_mode == CCFPmode || prev_mode == CCFPEmode)
+ && !(prev_code == EQ
+ || prev_code == NE
+ || prev_code == ORDERED
+ || prev_code == UNORDERED))
+ icode = code_for_ccmp_rev (cc_mode, cmp_mode);
+ else
+ {
+ rtx_code code = reverse_condition (prev_code);
+ prev = gen_rtx_fmt_ee (code, VOIDmode, XEXP (prev, 0), const0_rtx);
+ }
aarch64_cond = AARCH64_INVERSE_CONDITION_CODE (aarch64_cond);
}
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index eb1eca4..f46b01c 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -1026,12 +1026,10 @@ typedef struct
#define SELECT_CC_MODE(OP, X, Y) aarch64_select_cc_mode (OP, X, Y)
-#define REVERSIBLE_CC_MODE(MODE) 1
-
-#define REVERSE_CONDITION(CODE, MODE) \
- (((MODE) == CCFPmode || (MODE) == CCFPEmode) \
- ? reverse_condition_maybe_unordered (CODE) \
- : reverse_condition (CODE))
+/* Having an integer comparison mode guarantees that we can use
+ reverse_condition, but the usual restrictions apply to floating-point
+ comparisons. */
+#define REVERSIBLE_CC_MODE(MODE) ((MODE) != CCFPmode && (MODE) != CCFPEmode)
#define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) \
((VALUE) = GET_MODE_UNIT_BITSIZE (MODE), 2)
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index c0b7010..41dc4ed 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -493,16 +493,18 @@
""
"")
-(define_insn "ccmp<mode>"
- [(set (match_operand:CC 1 "cc_register" "")
- (if_then_else:CC
+(define_insn "@ccmp<CC_ONLY:mode><GPI:mode>"
+ [(set (match_operand:CC_ONLY 1 "cc_register" "")
+ (if_then_else:CC_ONLY
(match_operator 4 "aarch64_comparison_operator"
[(match_operand 0 "cc_register" "")
(const_int 0)])
- (compare:CC
+ (compare:CC_ONLY
(match_operand:GPI 2 "register_operand" "r,r,r")
(match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn"))
- (unspec:CC [(match_operand 5 "immediate_operand")] UNSPEC_NZCV)))]
+ (unspec:CC_ONLY
+ [(match_operand 5 "immediate_operand")]
+ UNSPEC_NZCV)))]
""
"@
ccmp\\t%<w>2, %<w>3, %k5, %m4
@@ -511,33 +513,57 @@
[(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
)
-(define_insn "fccmp<mode>"
- [(set (match_operand:CCFP 1 "cc_register" "")
- (if_then_else:CCFP
+(define_insn "@ccmp<CCFP_CCFPE:mode><GPF:mode>"
+ [(set (match_operand:CCFP_CCFPE 1 "cc_register" "")
+ (if_then_else:CCFP_CCFPE
(match_operator 4 "aarch64_comparison_operator"
[(match_operand 0 "cc_register" "")
(const_int 0)])
- (compare:CCFP
+ (compare:CCFP_CCFPE
(match_operand:GPF 2 "register_operand" "w")
(match_operand:GPF 3 "register_operand" "w"))
- (unspec:CCFP [(match_operand 5 "immediate_operand")] UNSPEC_NZCV)))]
+ (unspec:CCFP_CCFPE
+ [(match_operand 5 "immediate_operand")]
+ UNSPEC_NZCV)))]
"TARGET_FLOAT"
- "fccmp\\t%<s>2, %<s>3, %k5, %m4"
+ "fccmp<e>\\t%<s>2, %<s>3, %k5, %m4"
[(set_attr "type" "fccmp<s>")]
)
-(define_insn "fccmpe<mode>"
- [(set (match_operand:CCFPE 1 "cc_register" "")
- (if_then_else:CCFPE
+(define_insn "@ccmp<CC_ONLY:mode><GPI:mode>_rev"
+ [(set (match_operand:CC_ONLY 1 "cc_register" "")
+ (if_then_else:CC_ONLY
(match_operator 4 "aarch64_comparison_operator"
[(match_operand 0 "cc_register" "")
- (const_int 0)])
- (compare:CCFPE
+ (const_int 0)])
+ (unspec:CC_ONLY
+ [(match_operand 5 "immediate_operand")]
+ UNSPEC_NZCV)
+ (compare:CC_ONLY
+ (match_operand:GPI 2 "register_operand" "r,r,r")
+ (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn"))))]
+ ""
+ "@
+ ccmp\\t%<w>2, %<w>3, %k5, %M4
+ ccmp\\t%<w>2, %3, %k5, %M4
+ ccmn\\t%<w>2, #%n3, %k5, %M4"
+ [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
+)
+
+(define_insn "@ccmp<CCFP_CCFPE:mode><GPF:mode>_rev"
+ [(set (match_operand:CCFP_CCFPE 1 "cc_register" "")
+ (if_then_else:CCFP_CCFPE
+ (match_operator 4 "aarch64_comparison_operator"
+ [(match_operand 0 "cc_register" "")
+ (const_int 0)])
+ (unspec:CCFP_CCFPE
+ [(match_operand 5 "immediate_operand")]
+ UNSPEC_NZCV)
+ (compare:CCFP_CCFPE
(match_operand:GPF 2 "register_operand" "w")
- (match_operand:GPF 3 "register_operand" "w"))
- (unspec:CCFPE [(match_operand 5 "immediate_operand")] UNSPEC_NZCV)))]
+ (match_operand:GPF 3 "register_operand" "w"))))]
"TARGET_FLOAT"
- "fccmpe\\t%<s>2, %<s>3, %k5, %m4"
+ "fccmp<e>\\t%<s>2, %<s>3, %k5, %M4"
[(set_attr "type" "fccmp<s>")]
)
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 661c3e7..fc97308 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -22,6 +22,9 @@
;; Mode Iterators
;; -------------------------------------------------------------------
+;; Condition-code iterators.
+(define_mode_iterator CC_ONLY [CC])
+(define_mode_iterator CCFP_CCFPE [CCFP CCFPE])
;; Iterator for General Purpose Integer registers (32- and 64-bit modes)
(define_mode_iterator GPI [SI DI])
@@ -833,6 +836,9 @@
;; Mode attributes
;; -------------------------------------------------------------------
+;; "e" for signaling operations, "" for quiet operations.
+(define_mode_attr e [(CCFP "") (CCFPE "e")])
+
;; In GPI templates, a string like "%<w>0" will expand to "%w0" in the
;; 32-bit version and "%x0" in the 64-bit version.
(define_mode_attr w [(QI "w") (HI "w") (SI "w") (DI "x") (SF "s") (DF "d")])
diff --git a/gcc/testsuite/gcc.dg/torture/pr93133.c b/gcc/testsuite/gcc.dg/torture/pr93133.c
new file mode 100644
index 0000000..21eae1e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr93133.c
@@ -0,0 +1,41 @@
+/* { dg-do run } */
+/* { dg-add-options ieee } */
+/* { dg-require-effective-target fenv_exceptions } */
+/* { dg-skip-if "fenv" { powerpc-ibm-aix* } } */
+
+#include <fenv.h>
+
+int
+__attribute__ ((noinline, noclone))
+f1 (float a, float b)
+{
+ return -__builtin_isgreater (a, b);
+}
+
+int
+__attribute__ ((noinline, noclone))
+f2 (float a, float b)
+{
+ return -(a > b);
+}
+
+int
+main (void)
+{
+ volatile int r;
+
+ float nanf = __builtin_nanf ("");
+ float argf = 1.0f;
+
+ feclearexcept (FE_INVALID);
+
+ r = f1 (nanf, argf);
+ if (r != 0 || fetestexcept (FE_INVALID))
+ __builtin_abort ();
+
+ r = f2 (nanf, argf);
+ if (r != 0 || !fetestexcept (FE_INVALID))
+ __builtin_abort ();
+
+ return 0;
+}
More information about the Gcc-cvs
mailing list