Details
- Reviewers
nemanjai craig.topper kbarton jsji steven.zhang uweigand - Group Reviewers
Restricted Project
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
For unsigned conversion, the input is treated as signed here, and the result fixed up by adding 2^N; something along the lines of
(signed)N >= 0 ? signed_to_FP((signed)N) : signed_to_FP(N - 2^N) + (FP)2^N
For strict operation, we need to verify that the result is correct with any setting of the current rounding mode, and that the overall exception status after the two operations (signed_to_FP and the FADD) matches the intended semantics of the unsigned_to_FP operation. I believe in general this is not the case; specifically, there may be problems if the input (interpreted as signed) is not exactly representable as FP number,
If the input type is i32 or i64, that's probably not a problem, given that all such numbers are exactly representable as ppc_f128. However, if the input type is i128, this is no longer the case.
If you look at SelectionDAGLegalize::ExpandLegalINT_TO_FP, which uses a similar algorithm, you'll see that it switches to a different one in the case where not all inputs may be representable. I think we'll have to do something similar here for full correctness.
(OTOH given that this affects only the i128->ppc_f128 case, which should be relatively rare, and I think isn't even fully correct in all cases in non-strict mode even today, maybe this can be done later. But at least there should be a FIXME.)
See inline comments about chain handling.
| llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp | ||
|---|---|---|
| 1681 ↗ | (On Diff #278695) | This looks like the wrong input Chain if we already performed an operation above -- it should use the output chain of that operation as input here. |
| 1685 ↗ | (On Diff #278695) | OK, well, even if we keep the existing algorithm with the FIXME, this still isn't correct as it loses the Chain after the FADD. I think in the strict case we'll need to use a strict version of SelectCC (passing in the Chain from the FADD) and the use the resulting output Chain for the overall output (via ReplaceValueWith as above -- in fact, if we fall through down here, we actually should *not* do the ReplaceValueWith above, since that would be the wrong chain). |
| llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp | ||
|---|---|---|
| 1685 ↗ | (On Diff #278695) | Seems we don't have strict select_cc. Here Src is integer type, not suitable for fsetcc+select? And yes, I should replace original chain of N with newest chain, both here and inside isSigned if block. |
| llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp | ||
|---|---|---|
| 1685 ↗ | (On Diff #278695) | Ah, you're right -- this is an integer select, so there is no (need for a) strict version. So the only change needed is to do the chain replacement here, and inside the isSigned block (as you said). |