We legalize the ALU instructions for RISCV according to the hardware mode. W-instructions will be supported in a future patch.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I've added the previous patch legalizing ALU instructions to a child revision of this, since it also attempts to handle w-instructions. Our first major goal is implementing globalisel at -O0; w-instructions are not needed yet.
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
61 | Should we manually edit the formatting here? |
As long as you are not interested in vectors, I used this pattern:
getActionDefinitionsBuilder(G_FREEZE) .legalFor({s8, s16, s32, s64, p0}) .widenScalarToNextPow2(0, /*Min=*/8) .clampScalar(0, s8, sMaxScalar);
clamp to legal range and widen to next power of two to handle weird cases.
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
40 | I don't see any command lines with -mattr=+m, so this branch isn't tested? In fact there are no test cases for mul, udiv, urem, sdiv, or srem. |
Add tests for smaller sizes, mul/div/rem variants.
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
40 | You're right, I'll add these. |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
49 | What is the effect of the clampScalar? You are doing lib call for legal values? |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
47–48 | I don't think ordering libcall before anything else will work |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
53 | Is this libcall tested? There is no s128 libcall for RV32, but s64 should use a libcall on RV64. | |
77 | G_CONSTANT should only have 1 type. What does .maxScalar(1, XLenLLT); here do? | |
80 | How can G_ZEXT/G_SEXT/G_ANYEXT be legal for any types? The source and destination should be different types, and there is only 1 legal type on RISC-V. |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
80 | The most complete target for Gisel is AArch64. See there legalisation for EXTs: |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
80 | I think the AArch64 rules are over-permissive and old. It shouldn't be permitting <= register size, only == register size values through |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
80 | I found remark: unable to legalize instruction: %252:_(<16 x s32>) = G_ZEXT %249:_(<16 x s8>) [-Rpass-missed=gisel-legalize] in the wild. Till today, I have no clue how to legalize it on AArch64. |
Breaking the problem into smaller chunks seems helpful.
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
47–48 | I moved the libcall after the clampScalar, but retrospectively I think the original ordering was correct. getActionDefinitionsBuilder({G_MUL, G_SDIV, G_SREM, G_UDIV, G_UREM}) .legalFor({XLenLLT}) .libcallFor({s128}) .clampScalar(0, XLenLLT, XLenLLT); This reads as "These opcodes are legal for XLenLLT. If that check fails, see if the type is s128 and then use a libcall. If that fails, try extending the argument 0 to XLenLLT type." That seems correct to me. | |
49 | If I understand correctly, this is being addressed by the conversation with arsenm above. |
I don't see any command lines with -mattr=+m, so this branch isn't tested?
In fact there are no test cases for mul, udiv, urem, sdiv, or srem.