[PATCH] D126644: [llvm/CodeGen] Add ExpandLargeDivRem pass

[Matthias Gehre via Phabricator via llvm-commits]

mgehre-amd added inline comments.


================
Comment at: llvm/lib/CodeGen/ExpandLargeDivRem.cpp:12
+// This is useful for backends like x86 that cannot lower divisions
+// with more than 128 bits.
+//
----------------
craig.topper wrote:
> I think but haven't checked that 32-bit x86, ARM, RISCV32, and other 32-bit targets can't lower division with more than 64 bits.
I checked x86_32 and it lowers 128 bit divisions to __divti3: https://godbolt.org/z/Y6hT9n3x5


================
Comment at: llvm/lib/CodeGen/ExpandLargeDivRem.cpp:33
+    ExpandDivRemBits("expand-div-rem-bits", cl::Hidden, cl::init(128),
+                     cl::desc("div and rem instructions on integers with <N> "
+                              "or more bits are expanded."));
----------------
craig.topper wrote:
> This says "<N> or more bits", but the code exits for `<= ExpandDivRemBits`
done


================
Comment at: llvm/lib/Transforms/Utils/IntegerDivision.cpp:145
 
-  ConstantInt *Zero;
-  ConstantInt *One;
-  ConstantInt *NegOne;
-  ConstantInt *MSB;
-
-  if (BitWidth == 64) {
-    Zero      = Builder.getInt64(0);
-    One       = Builder.getInt64(1);
-    NegOne    = ConstantInt::getSigned(DivTy, -1);
-    MSB       = Builder.getInt64(63);
-  } else {
-    assert(BitWidth == 32 && "Unexpected bit width");
-    Zero      = Builder.getInt32(0);
-    One       = Builder.getInt32(1);
-    NegOne    = ConstantInt::getSigned(DivTy, -1);
-    MSB       = Builder.getInt32(31);
-  }
+  ConstantInt *Zero      = Builder.getIntN(BitWidth, 0);
+  ConstantInt *One       = Builder.getIntN(BitWidth, 1);
----------------
craig.topper wrote:
> You already have the DivTy, why not go through ConstantInt::get for all of these?
done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126644