[PATCH] D110890: [GlobalISel] Port the udiv -> mul by constant combine.

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/Utils.h:400
 
+/// Determines if \p MI defines a constant integer or a splat/build vector of
+/// constant integers. Treats undef values as constants.
----------------
Seems odd to mention "splat" at all here.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4420
+  LLT ScalarShiftAmtTy = ShiftAmtTy.getScalarType();
+  auto &MIB = Builder;
+  MIB.setInstrAndDebugLoc(MI);
----------------
What's this for?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4561-4562
+
+    // For vectors we might have a mix of non-NPQ/NPQ paths, so use
+    // G_UMULH to act as a SRL-by-1 for NPQ, else multiply by zero.
+    if (Ty.isVector())
----------------
aemerson wrote:
> foad wrote:
> > Does the umulh really work out cheaper than (say) shifting NPQ right by one, and then ANDing that with a mask of -1s and 0s?
> I think we can implement another combine to optimize the G_MULHU like the DAG does in this case?
Well D111036 as it stands does not go far enough. It does not handle non-splat constant vectors, and it does not handle zeros in vectors.

I still think using G_UMULH and relying on it being cleaned up later is a strange implementation choice, but I guess if D111036 can be improved then it's OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110890