This patch adds support for constrained conversion operation (fptoui/fptosi) from f32/f64 to i32/i64.
Vector support will be done in following patches. For targets older than ISA 2.06, we need to make strict_fsetcc/strict_fsetccs work well first.
Paths
| Differential D81537
[PowerPC] Support constrained fp operation for scalar fptosi/fptoui ClosedPublic Authored by qiucf on Jun 10 2020, 12:27 AM.
Details
Summary This patch adds support for constrained conversion operation (fptoui/fptosi) from f32/f64 to i32/i64. Vector support will be done in following patches. For targets older than ISA 2.06, we need to make strict_fsetcc/strict_fsetccs work well first.
Diff Detail
Event Timeline
qiucf added a child revision: D81669: [PowerPC] Support constrained fp operation for scalar sitofp/uitofp.Jun 11 2020, 9:48 AM
qiucf added a parent revision: D81818: [NFC] [PowerPC] Use shared method in FP_TO_INT and INT_TO_FP lowering.Jun 14 2020, 10:32 PM
nemanjai added inline comments.
qiucf added inline comments.
qiucf marked an inline comment as done. Comment Actions
qiucf added inline comments. qiucf marked an inline comment as done. Comment ActionsReflect changes after strict conversion for SPE and enable-ppc-quad-precision's removal. This revision is now accepted and ready to land.Aug 4 2020, 2:34 AM Comment Actions This doesn't look correct. As far as I can see, none of the conversion functions were actually changed to handle strict operations. For one, you'll need strict variants of all the PowerPC-specific conversion operations, use them in all the conversion subroutines, and consistently track their chain nodes. The patch only adds a strict variant of the direct move, which seems to me the only operation where actually a strict version is not required ...
Comment Actions
Thanks for pointing them out! I have something unclear about chains: (1) If a constrained operation is expanded into several FP nodes a-b-c, they should all have chain set to former operation (b's chain is a, c's chain is b) even if they have def relationship? (2) In MachineInstr emitting after ISel, chains are identified just by operand type (countOperand), so some chains are not ignored and assert hit. Is this expected?
Comment Actions
That may depend on the specific semantics on which of those nodes may or may not trap. In some cases, the original sequence may in fact not be valid at all for strict mode. But if it is, then they'll need to be chained up properly. If they have data dependencies, then it usually makes sense for the chain to follow that dependency. In other cases, the may be an option for more flexibility by allowing certain operations to be re-scheduled. In those cases you'd give the same input chain to all operations and collect all output chains via a TokenFactor.
I'm not sure I understand what specific case you're refering to. But in any case, a chain should *never* be simply ignored, that would always be a bug.
Comment Actions Thanks for the detailed explanation!
qiucf marked 2 inline comments as done. Comment Actions
Closed by commit rG131b3b9ed4ef: [PowerPC] Support constrained scalar fptosi/fptoui (authored by qiucf). · Explain WhyAug 19 2020, 10:35 PM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 270025 llvm/lib/Target/PowerPC/PPCISelLowering.h
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
llvm/lib/Target/PowerPC/PPCInstrSPE.td
llvm/lib/Target/PowerPC/PPCInstrVSX.td
llvm/test/CodeGen/PowerPC/fp-strict-conv-f128.ll
llvm/test/CodeGen/PowerPC/fp-strict-conv.ll
|
Why? The instruction simply moves bits around. It does not cause any exceptions, it is not subject to rounding, etc. If this is necessary, it needs to be clear from the comment why.