This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Support vector strict rounding operations.
ClosedPublic

Authored by fakepaper56 on Apr 17 2023, 6:06 AM.

Details

Summary

The patch basically models custom lowering of base rounding operations to expand
rounding by coverting to ingter and coverting back to FP. The other one thing
the patch does is to covert sNan of the source to qNan.

Diff Detail

Event Timeline

fakepaper56 created this revision.Apr 17 2023, 6:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 17 2023, 6:06 AM
fakepaper56 requested review of this revision.Apr 17 2023, 6:06 AM
craig.topper added inline comments.Apr 17 2023, 3:23 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2578

There shouldn't be an overflow exception. If the value is already an "integer" in the FP register we shouldn't go through the integer conversion. The integer conversion only exists because of an oddity of the RISC-V ISA. That's not part of the definition of these functions.

The one thing we need to do that the non-strict doesn't handle is converting snan to qnan.

craig.topper added inline comments.Apr 17 2023, 3:26 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2578

And we need to avoid raising an inexact exception for nearbyint.

fakepaper56 added inline comments.Apr 17 2023, 6:04 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2578

The one thing we need to do that the non-strict doesn't handle is converting snan to qnan.

Sorry, I don't know why we need to covert snan to qnan? Could you elaborate it ?

craig.topper added inline comments.Apr 17 2023, 7:36 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2578

A signaling nan input should set the invalid exception and produce a quiet nan output. I believe the the glibc library implementation handles it by adding the input to itself if the input is nan. The fadd instruction produces qnan for an snan input and raises the invalid exception.

fakepaper56 added inline comments.Apr 17 2023, 7:44 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2578

Thank you. I got your idea.

Expanding by converting sNan to qNan and using base opcocdes with the new source.
The change makes the revision be blocked by D148619, a revision supports
vector rint/nearbyint in RISC-V.

fakepaper56 planned changes to this revision.Apr 18 2023, 2:29 AM
fakepaper56 edited the summary of this revision. (Show Details)
craig.topper added inline comments.Apr 18 2023, 3:50 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
402

STRICT_FNEARBYINT is already the third entry in the list

2556

Src needs a freeze.

2562

This needs to be a masked fadd I think. If Src is a large number instead of a nan, adding a larger number to itself can produce infinity which will set the overflow exception which we don't want.

2589

I'm not sure we can convert to the base opcode. I think we need to keep the chain through the expanded opcodes. We need to make sure rounding mode changes or reads of fflags that should logically happen after the rounding expansion can't move up. The fadd probably protects anything above from sinking down.

fakepaper56 added inline comments.Apr 18 2023, 7:22 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2589

Is the strict_nearbyint is the cause that we should not covert to base opcode? If some modification of FRM is after strict_nearbyint and we covert strict_nearbyint to nearbyint, it maybe let compiler move the modification of FRM before the nearbyint.

Just expanding by coverting to integer and coverting back to FP and also
coverting sNan to qNan.

fakepaper56 edited the summary of this revision. (Show Details)Apr 19 2023, 12:10 AM
fakepaper56 marked 3 inline comments as done.Apr 19 2023, 12:17 AM
craig.topper added inline comments.Apr 19 2023, 12:17 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2589

That is one case yes. I think we could also move a write of fflags before the integer conversion. The integer can cause an inexact exception. If the user was trying to clear that inexact exception, it would be a bug to move their fflag write earlier.

craig.topper added inline comments.Apr 19 2023, 12:27 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2636

Update the Chain in this if too.

Update chain after coverting back to FP.

This revision is now accepted and ready to land.Apr 26 2023, 10:17 AM
This revision was landed with ongoing or failed builds.Apr 26 2023, 8:35 PM
This revision was automatically updated to reflect the committed changes.