This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add strictfp support for compares.
ClosedPublic

Authored by craig.topper on Jan 5 2022, 1:18 PM.

Details

Summary

This adds support for STRICT_FSETCC(quiet) and STRICT_FSETCCS(signaling).

FEQ matches well to STRICT_FSETCC oeq.
FLT/FLE matches well to STRICT_FSETCCS olt/ole.

Others require commuting operands or multiple instructions.

STRICT_FSETCC olt/ole/ogt/oge/ult/ule/ugt/uge uses FLT/FLE,
but we need to save/restore FFLAGS around them to avoid spurious
exceptions. I've implemented pseudo instructions with a
CustomInserter to insert the save/restore CSR instructions.
Unfortunately, this doesn't honor exceptions for signaling NANs
but I'm not sure if signaling nans are really supported by the
constrained intrinsics.

STRICT_FSETCC one and ueq expand to a pair of FLT instructions
with a save/restore of fflags around each. This could be improved
in the future.

There may be some opportunities to generate better code for strict
comparisons mixed with nonans fast math flags. I've left FIXMEs in
the .td files for that.

Co-Authored-by: ShihPo Hung <shihpo.hung@sifive.com>

Diff Detail

Event Timeline

craig.topper created this revision.Jan 5 2022, 1:18 PM
craig.topper requested review of this revision.Jan 5 2022, 1:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 5 2022, 1:18 PM
Herald added a subscriber: MaskRay. · View Herald Transcript

Forgot to git add the half test. Will add soon.

Add half test.

@andrew.w.kaylor or @kpn can you answer what whether quiet compares should signal for SNAN. Clang ignores gcc's -fsignaling-nans option so I'm not sure what level of SNAN support we have is.

kpn added a comment.Jan 5 2022, 3:07 PM

In IRBuilder.h we have this:

// Create a signaling floating-point comparison (i.e. one that raises an FP
// exception whenever an input is any NaN, signaling or quiet).
// Note that this differs from CreateFCmp only if IsFPConstrained is true.
Value *CreateFCmpS(CmpInst::Predicate P, Value *LHS, Value *RHS,

and

// Create a quiet floating-point comparison (i.e. one that raises an FP
// exception only in the case where an input is a signaling NaN).
// Note that this differs from CreateFCmpS only if IsFPConstrained is true.
Value *CreateFCmp(CmpInst::Predicate P, Value *LHS, Value *RHS,

So I think the answer to your question is "yes" based on IRBuilder.h.

Preserve NoFPExcept flag when expanding the pseudo. Though maybe NoFPExcept gives permission to not save/restore FFLAGS?
Add an FEQ to the NonSignaling pseudo expansion to signal for SNANs.

clang-format
Rename NonSignal->Quiet

arcbbb accepted this revision.Jan 11 2022, 2:12 AM

LGTM. Thanks Craig for improving this!

This revision is now accepted and ready to land.Jan 11 2022, 2:12 AM

Add -target-abi to tests.

This revision was automatically updated to reflect the committed changes.
Jim added inline comments.Oct 30 2022, 11:42 PM
llvm/test/CodeGen/RISCV/double-fcmp-strict.ll
205

I am not sure that does the exception raise in __ledf2 ?
In GCC, it use __unorddf2 to check either operands is NaN before __ledf2.

craig.topper added inline comments.Oct 31 2022, 12:17 AM
llvm/test/CodeGen/RISCV/double-fcmp-strict.ll
205

I'm not seeing __unorddf2 when I tried testing https://godbolt.org/z/fb7PWaso4

Jim added inline comments.Oct 31 2022, 4:03 AM
llvm/test/CodeGen/RISCV/double-fcmp-strict.ll
205

https://godbolt.org/z/nWbfh9hjx
Clang extra appended options -ffp-exception-behavior=strict -Xclang -fexperimental-strict-floating-point to enable strict floating-point.

option: -O3 -march=rv64if -mabi=lp64f -g0 -ffp-exception-behavior=strict -Xclang -fexperimental-strict-floating-point

addi    sp, sp, -16
sd      ra, 8(sp)                       # 8-byte Folded Spill
call    __gtdf2@plt                                                              <<< may raise fp exception
sgtz    a0, a0
ld      ra, 8(sp)                       # 8-byte Folded Reload
addi    sp, sp, 16
ret

option: -O3 -march=rv64ifd -mabi=lp64f -g0 -ffp-exception-behavior=strict -Xclang -fexperimental-strict-floating-point

fmv.d.x ft0, a0
fmv.d.x ft1, a1
frflags a1
flt.d   a0, ft1, ft0
fsflags a1
feq.d   zero, ft1, ft0
ret

GCC use call __unorddf2 to check either operands is NaN.

craig.topper added inline comments.Oct 31 2022, 4:53 PM
llvm/test/CodeGen/RISCV/double-fcmp-strict.ll
205

Sorry I misunderstood which comparison we were talking about and what you thought the bug was.

I agree this looks like a bug. I'm not how to fix. We probably need to introduce control flow in IR before SelectionDAG to branch over the call to __ledf2

Are there any builtin functions that can generate llvm.experimental.constrained.fcmp?

Are there any builtin functions that can generate llvm.experimental.constrained.fcmp?

It supposed to be generated by >=, <=, >, <, ==, != on floating point types when -frounding-math or -ftrapping-math or a few other options are passed to clang, but those options are rejected because constrained intrinsics aren't handled properly for vectors yet. I once proposed enabled enabling it if vectors weren't enabled, https://reviews.llvm.org/D130311