On 9/16/26 6:12 AM, Matt Turner wrote:
A target defines the isnan, isinf and isnormal optabs when FP comparisons
are unsuitable for classification, and fold_builtin_interclass_mathfn
leaves those built-ins alone when it does. fold_builtin_fpclassify does
not: it always expands into comparisons, so __builtin_fpclassify keeps
the problems the individual built-ins were fixed to avoid. On
AArch64 with -fsignaling-nans, where all three optabs are now available,
fpclassify still raises FE_INVALID for a signaling NaN, as it does on
LoongArch, which defines the same three optabs under that flag; on Alpha
without -mieee a comparison against a subnormal operand traps or flushes
it to zero, so FP_SUBNORMAL cannot be told from FP_ZERO.
When all three optabs are available for the mode, build fpclassify on the
corresponding built-ins instead, and test zero versus subnormal by shifting
the sign bit out of the encoding rather than by comparing against zero,
which would reintroduce the same problem. That shift is only valid for a
format whose sign bit is the top bit of the matching integer mode, so
formats like XFmode, whose encoding is narrower than that mode, keep the
generic expansion.
With -fsignaling-nans on AArch64, fpclassify of a double becomes:
fmov x1, d0
mov x2, -9007199254740992
mov w0, 0
cmp x2, x1, lsl 1
bcc .L1
mov w0, 1
cmp x2, x1, lsl 1
beq .L1
ubfx x2, x1, 52, 11
mov w0, 4
sub x2, x2, #1
cmp x2, 2045
bls .L1
cmp xzr, x1, lsl 1
cset w0, ne
add w0, w0, 2
.L1:
ret
gcc/ChangeLog:
PR middle-end/66462
* builtins.cc (fold_builtin_fpclassify): Expand using the
classification built-ins when their optabs are available.
gcc/testsuite/ChangeLog:
PR middle-end/66462
* gcc.target/aarch64/pr66462-2.c: New test.
* gcc.target/alpha/fp-classify-5.c: New test.
* gcc.target/alpha/fp-classify-6.c: New test.
The series looks good to me. Just one small concern.
You check signbit_rw to verify location of the sign bit so that you can
shift it out with a single bit left shift. It seems like you want to
also verify that signbit_rw == signbit_ro. It's perhaps a bit
over-cautious, but you can see other code doing the same kind of sanity
check.
Thoughts?
jeff
