Legalize multiplication with the +m, +zmmul extensions and without extensions. With extensions, we test for (s7, s8, s16, s32, s48, s64, s96) on rv32 and (s8, s15, s32, s64, s72, s128, s192) on rv64. Without extensions, test (s7, s8, s16, s32) on rv32 and (s8, s15, s16, s32, s64) on rv64. Does not yet work for the type which is 2 times XLen without extensions.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
95 | Why did we stray from the usual formatting here? | |
llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-mul-ext.mir | ||
6 | Can +m and +zmmul share the same check prefix? | |
11 | CHECK-I is not a valid prefix for this test. | |
llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-mul-ext.mir | ||
4 | Check prefixes in this test have an I in them that the rv32 tests don't have. We should be consistent |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
95 | clang-format does this. You can see similar formatting on line 71. Should I manually format it with // clang-format off tags? |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
95 | I support turning off clang-format here |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
95 | Why do we need .lower() on this? We have instructions for G_SMULH and G_UMULH. |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
95 | I would assume you want some clamping before going direct to lower (although I wouldn't be surprised if there wasn't complete support for mulh lowering) |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
95 | Ignore my comment. I think I missed the legalFor |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
90 | This builder looks the same with and without the extension? Could you hoist it above the if? |
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
90 | Is the MULO part of this tested? |
Fixed formatting and deleted legalization for G_(U/S)MULO. G_(U/S)MULO needs to be legalized in another patch with its own tests.
llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp | ||
---|---|---|
101 | I assume MULO here isn't needed/tested either? |
This builder looks the same with and without the extension? Could you hoist it above the if?