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.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2572 | 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. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2572 | And we need to avoid raising an inexact exception for nearbyint. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2572 |
Sorry, I don't know why we need to covert snan to qnan? Could you elaborate it ? |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2572 | 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. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2572 | 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.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
401 | STRICT_FNEARBYINT is already the third entry in the list | |
2550 | Src needs a freeze. | |
2556 | 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. | |
2583 | 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. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2583 | 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.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2583 | 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. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2630 | Update the Chain in this if too. |
STRICT_FNEARBYINT is already the third entry in the list