mov w8, #1325400064 + fmov s0, w8 ==> movi v0.2s, 0x4f, lsl 24
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Using the MOVI to materialize fp constants sounds like a good idea.
But I don't think this should be starting from a CopyToReg. CopyToReg usually means we are copying the result into the return value of a function (or into a call argument). Copying into s0 for example. CopyFromReg is used to get the value from input physical registers from arguments.
An example like this, with fadd, would preferably use the movi lowering too: https://godbolt.org/z/bc99E85qz
If the FPConstant is still in the DAG at the point that we run instruction selection (I think it should be but it may be dependant on what AArch64TargetLowering::isFPImmLegal returns), then it may be best to add a tablegen pattern for cases that we can materialize with the integer movi instruction. From here: https://github.com/llvm/llvm-project/blob/ecff9b65b54c7a4bd79ca2af157c81595678f0ee/llvm/lib/Target/AArch64/AArch64InstrInfo.td#L1545, with a different fpimm to limit it to cases where the MOVi can be generated, and an EXTRACT_SUBREG to get the s reg. If that doesn't work (the custom ImmLeaf's might be a bit much) then perhaps something in AArch64ISelDAGToDAG.cpp?
llvm/test/Transforms/LoopVectorize/AArch64/remat-const-float-simd.ll | ||
---|---|---|
2 ↗ | (On Diff #411754) | It looks like the test is completely unrelated to the loop-vectorize pass. It should probably be either moved to CodeGen/AArch64/ or added to one of the existing tests there. |
llvm/lib/Target/AArch64/AArch64InstrFormats.td | ||
---|---|---|
1210 | This likely doesn't need to be an Operand, it can just be a FPImmLeaf | |
llvm/lib/Target/AArch64/AArch64InstrInfo.td | ||
1551 | This doesn't need to define a new instruction, it can use the existing MOVI (which I believe is probably called MOVIv2i32?) | |
llvm/test/Transforms/LoopVectorize/AArch64/remat-const-float-simd.ll | ||
1 ↗ | (On Diff #411754) | This test file should be in CodeGen/AArch64 |
a) move case remat-const-float-simd.ll to CodeGen/AArch64
b) use MOVIv2i32 , and delete unneeded instruction MOVI2s_ns
c) delete Operand<f32>, and just be a FPImmLeaf
Address all comment
llvm/lib/Target/AArch64/AArch64InstrFormats.td | ||
---|---|---|
1210 | Yes, verified ok! | |
llvm/lib/Target/AArch64/AArch64InstrInfo.td | ||
1551 | It's strange that here error with the following info, as it is ok on my local linux ?
| |
1551 | Thanks, delete unneeded instruction MOVI2s_ns |
Thanks for the updates. There are other forms of MOVI that might also be useful for some fp immediates. The one we have here (shift an i8 by 24) I imagine is the most useful, but the others could be good in places too. The MOVN's too. I'm not sure what the best way to generalise this would be. As far as I can tell it needs lots of checks to the various isAdvSIMDModImmTypeXYZ methods wherever it is? This seems like a good beginning at least.
llvm/lib/Target/AArch64/AArch64InstrFormats.td | ||
---|---|---|
1210 | It's likely worth calling this out as a "AdvSIMDModImmType4" constant somehow. Maybe call it fpimm32SIMDModImmType4? Same for the XForm. | |
llvm/lib/Target/AArch64/AArch64InstrInfo.td | ||
6149 | It is quite uncommon to not have NEON, but can you add a predicate for it: let Predicates = [HasNEON] in { It might be worth adding a run line without neon (-mattr=-neon) for the new remat test case too, to show the difference. | |
llvm/test/CodeGen/AArch64/remat-const-float-simd.ll | ||
8 | 0x7fffffff -> 2147483648, as the 0x7fffffff gets rounded. |
This likely doesn't need to be an Operand, it can just be a FPImmLeaf