[PATCH] D130517: [GlobalISel] Add sdiv exact (X, constant) -> mul combine.

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4964
+    auto CheckEltValue = [&](const Constant *C) {
+      if (auto *CI = dyn_cast_or_null<ConstantInt>(C))
+        return !CI->isZero();
----------------
Just `return C && !C->isNullValue()`?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:5005
+
+    // Calculate the multiplicative inverse, using Newton's method.
+    APInt t;
----------------
Use APInt::multiplicativeInverse instead?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:5022
+  auto *RHSDef = cast<GenericMachineInstr>(getDefIgnoringCopies(RHS, MRI));
+  if (isa<GBuildVector>(RHSDef)) {
+    Shift = MIB.buildBuildVector(ShiftAmtTy, Shifts).getReg(0);
----------------
Is this a way of testing whether the divisor is //not// a splat? It seems a bit hacky. If the divisor is a splat then we could have saved some work above by only computing the multiplicative inverse once.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:5026
+  } else {
+    Shift = Shifts[0];
+    Factor = Factors[0];
----------------
How does this work? It seems like it would use a scalar shift amount for a vector shift, which is not allowed by MachineVerifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130517