When Zbt is enabled, we can generate SELECT for division by power
of 2, so that there is no data dependency.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
10357 | Capitalize 'only' | |
10367 | Is i64 correct here for RV32? I don't see a test case. | |
10368 | Use APInt::isNegatedPowerOf2 | |
llvm/test/CodeGen/RISCV/rv32zbt-div-pow2.ll | ||
1 ↗ | (On Diff #390952) | Can you add test cases for dividing by 2 and -2 as well? X86 does not use BuildSDIVPow2 for those cases so we should check those. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
10358 | Should say *why*, ie need conditional move | |
10362 | X86 prioritises isIntDivCheap over a lack of conditional move; what's the justification for doing it the other way round? | |
10368 | X86 asserts it's one of the two, so this check seems bogus given the function name... | |
llvm/test/CodeGen/RISCV/rv32zbt-div-pow2.ll | ||
7 ↗ | (On Diff #390952) | No positive power of 2 tests, and no tests for isIntDivCheap being true? |
llvm/test/CodeGen/RISCV/rv64zbt-div-pow2.ll | ||
8 ↗ | (On Diff #390952) | Don't duplicate tests, just add rv64 check lines to your existing rv32-only test |
31 ↗ | (On Diff #390952) | And we should test we don't do stupid things for i64 on RV32 |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
10362 | I agree with @jrtc27. The isIntDivCheap check should be at the top. | |
10368 | I think AArch64 explicitly checks it. I wrote X86 later and used an assert. | |
llvm/test/CodeGen/RISCV/rv32zbt-div-pow2.ll | ||
7 ↗ | (On Diff #390952) | isIntDivCheap is never true for RISCV today. We should also add a test for dividing by 4096 or larger for RV32 and RV64. And probably something larger than 2^32 for RV64. The constant materialization will add additional instructions not seen in the 1024 case. |
That's what several targets use isIntDivCheap for. I don't think RISCV does. The patch here doesn't make that situation worse.
Some of the i32 ones look like they might not be profitable on RV64 unless you have a wide out-of-order processor; 50% more instructions in some cases
For i32 on RV64, sign extension is needed, so there are sext.w. And the only case with 50% more instructions is when divisor is 2.
Maybe we should skip these divisors, any ideas?
llvm/test/CodeGen/RISCV/rv32zbt-div-pow2.ll | ||
---|---|---|
7 ↗ | (On Diff #390952) |
|
7 ↗ | (On Diff #390952) | Thanks. I added tests divided by n (|n| ≥ 2^12) and two materialization instructions were generated. So it may be non-profitable when absolute values of divisors are larger than 2^12. |
From the LLVM Code-Review Policy and Practices (https://llvm.org/docs/CodeReview.html):
If it is urgent, provide reasons why it is important to you to get this patch landed and ping it every couple of days. If it is not urgent, the common courtesy ping rate is one week. Remember that you’re asking for valuable time from other professional developers.
It's only been 3 days for this and other patches you've just pinged
Capitalize 'only'