On 7/12/24 3:12 PM, Vineet Gupta wrote:
Changes since v2:
- fclass define_insn tightened to check op0 mode "X" with additional
expander w/o mode for callers.
- builtins expander explicit mode check and FAIL if mode not appropriate.
- subreg promoted handling to elide potential extension of ret val.
- Added isinf builtin with bimodal retun value as others.
Changes since v1:
- Removed UNSPEC_{INFINITE,ISNORMAL}
- Don't hardcode SI in patterns, try to keep X to avoid potential
sign extension pitfalls. Implementation wise requires skipping
:MODE specifier in match_operand which is flagged as missing mode
warning.
---
Currently thsse builtins use float compare instructions which require
FP flags to be save/restore around them.
Our perf team complained this could be costly in uarch.
RV Base ISA already has FCLASS.{d,s,h} instruction to compare/identify FP
values w/o disturbing FP exception flags.
Coincidently, upstream very recently got support for the corresponding
optabs. So this just requires wiring up in the backend.
Tested for rv64, one additioal failure g++.dg/opt/pr107569.C needs
upstream ranger fix for the new optab.
gcc/ChangeLog:
* config/riscv/riscv.md: Add UNSPEC_FCLASS.
define_insn and define_expand for fclass.
define_expand for isfinite, isnormal, isinf.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/fclass.c: New tests.
Signed-off-by: Vineet Gupta <vine...@rivosinc.com>
---
gcc/config/riscv/riscv.md | 134 ++++++++++++++++++++++++
gcc/testsuite/gcc.target/riscv/fclass.c | 38 +++++++
2 files changed, 172 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/riscv/fclass.c
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index ff37125e3f28..c67e5129753a 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -68,6 +68,7 @@
UNSPEC_FMAX
UNSPEC_FMINM
UNSPEC_FMAXM
+ UNSPEC_FCLASS
;; Stack tie
UNSPEC_TIE
@@ -3436,6 +3437,139 @@
(set_attr "mode" "<UNITMODE>")
(set (attr "length") (const_int 16))])
+;; fclass instruction output bitmap
+;; 0 negative infinity
+;; 1 negative normal number.
+;; 2 negative subnormal number.
+;; 3 -0
+;; 4 +0
+;; 5 positive subnormal number.
+;; 6 positive normal number.
+;; 7 positive infinity
+;; 8 signaling NaN.
+;; 9 quiet NaN
+
+(define_insn "*fclass<ANYF:mode><X:mode>"
+ [(set (match_operand:X 0 "register_operand" "=r")
+ (unspec [(match_operand:ANYF 1 "register_operand" " f")]
+ UNSPEC_FCLASS))]
+ "TARGET_HARD_FLOAT"
+ "fclass.<fmt>\t%0,%1";
+ [(set_attr "type" "fcmp")
+ (set_attr "mode" "<UNITMODE>")])
+
+;; Implementation note:
+;; Indirection via the expander needed due to md syntax limitations.
+;; define_insn needs tighter mode check (X for operand0) requiring <X:mode>
+;; in name. This forces callers like isfinite to specify mode but can't due
+;; to their own standard pattern name requirement. The additional expander
+;; with matching RTL template (but elided mode check) allows callers to use
+;; expander's name with actual asm coming from stricter define_insn.
+
+(define_expand "fclass<ANYF:mode>"
+ [(set (match_operand 0 "register_operand" "=r")
+ (unspec [(match_operand:ANYF 1 "register_operand" " f")]
+ UNSPEC_FCLASS))]
+ "TARGET_HARD_FLOAT")
Note that for fclass, which isn't a standard pattern name, you have
additional text in the same -- say for example a mode. It's just those
standard names defined in md.texi where we have naming restrictions.
The net is you can avoid the fclass expander entirely. So you can drop
the '*' in the fclass pattern. Then call it as
gen_fclass<ANYF:mode>di or gen_fclass<ANYF:mode>si in the is* expanders.
+ /* Allow SI for rv32/rv64 and DI for rv64
+ Explicit mode check (vs. specfying modes in RTL template match_operand)
+ due to syntax limitations.
+ - Specifying X would cause multiple definitions
+ - And to disambuguate that requires adding <X:mode> to name which
+ no longer matches the "standard pattern name". */
+ if (GET_MODE (operands[0]) != SImode
+ && GET_MODE (operands[0]) != word_mode)
+ FAIL;
+
+ rtx t = gen_reg_rtx (word_mode);
+ rtx t_op0 = gen_reg_rtx (word_mode);
+
+ emit_insn (gen_fclass<ANYF:mode> (t, operands[1]));
+ riscv_emit_binary (AND, t, t, GEN_INT (0x7e));
+ rtx cmp = gen_rtx_NE (word_mode, t, const0_rtx);
+ emit_insn (gen_cstore<mode>4 (t_op0, cmp, t, const0_rtx));
+
+ if (TARGET_64BIT)
+ {
+ t_op0 = gen_lowpart (SImode, t_op0);
+ SUBREG_PROMOTED_VAR_P (t_op0) = 1;
+ SUBREG_PROMOTED_SET (t_op0, SRP_SIGNED);
+ }
+
+ emit_move_insn (operands[0], t_op0);
+ DONE;
+})
So this is repeated nearly verbatim in all 3 is* expanders. Seems ripe
for refactoring where you pass in the magic constant (which is all that
seems to change across those code fragments).
Hate to ask for another spin, but the refactoring seems like its worth
doing to avoid the duplication. And if we're going to spin again, might
as well drop the fclass expander that we really don't need.
Jeff
