[PATCH] D114856: [RISCV] Override TargetLowering::BuildSDIVPow2 to generate SELECT

[wangpc via Phabricator via llvm-commits]

pcwang-thead marked 13 inline comments as done.
pcwang-thead added a comment.

In D114856#3188850 <https://reviews.llvm.org/D114856#3188850>, @jrtc27 wrote:

> 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?



================
Comment at: llvm/test/CodeGen/RISCV/rv32zbt-div-pow2.ll:7
+
+define i32 @sdiv_pow2(i32 %a) {
+; RV32I-LABEL: sdiv_pow2:
----------------
craig.topper wrote:
> jrtc27 wrote:
> > No positive power of 2 tests, and no tests for isIntDivCheap being true?
> 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.
> 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.




================
Comment at: llvm/test/CodeGen/RISCV/rv32zbt-div-pow2.ll:7
+
+define i32 @sdiv_pow2(i32 %a) {
+; RV32I-LABEL: sdiv_pow2:
----------------
pcwang-thead wrote:
> craig.topper wrote:
> > jrtc27 wrote:
> > > No positive power of 2 tests, and no tests for isIntDivCheap being true?
> > 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.
> > 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.
> 
> 
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114856/new/

https://reviews.llvm.org/D114856