[PATCH] D96210: [TargetLowering][RISCV][AArch64][PowerPC] Enable BuildUDIV/BuildSDIV on illegal types before type legalization if we can find a larger legal type that supports MUL.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5101
+      if (NVT.getSizeInBits() >= (2 * EltBits) &&
+          isOperationLegal(ISD::MUL, NVT)) {
+        MulVT = NVT;
----------------
RKSimon wrote:
> Do we not have a legalization helper function that we can use?
I thought about using getTypeAction() == TypePromoteInteger and getTypeToTransformTo. And now I can't remember why I didn't.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5187
+          DAG.getConstant(EltBits, dl,
+                          getShiftAmountTy(MulVT, DAG.getDataLayout())));
+      return DAG.getNode(ISD::TRUNCATE, dl, VT, Y);
----------------
RKSimon wrote:
> Pre-legalization shift amount is always nasty - is there any chance that this can be out of range?
MulVT here is a legal type so the shift should be legal. So I think the target better be returning a shift amount type that should cover all possible amounts.


================
Comment at: llvm/test/CodeGen/AArch64/urem-seteq.ll:86
+; CHECK-NEXT:    mov w9, #14
+; CHECK-NEXT:    msub w8, w8, w9, w0
+; CHECK-NEXT:    tst w8, #0xffff
----------------
RKSimon wrote:
> No idea why but its odd that we now match a multiply-subtract here but lose the multiply-add in urem-seteq-nonzero.ll above
The msub here is part of the remainder calculation. The case above used a divisor of 3 which is one more than a power of 2 so the multiply for the remainder was turned into "add w8, w8, w8, lsl #1"

We're using different algorithms now so its hard to say much more than that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96210