[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/Target/GlobalISel/Combine.td:692
+def udiv_by_const : GICombineRule<
+  (defs root:$root, apint_matchinfo:$info),
+  (match (wip_match_opcode G_UDIV):$root,
----------------
Do you need $info here? It looks unused.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4429
+/// of the divided value are known zero.
+static mu magicu(const APInt &d, unsigned LeadingZeros = 0) {
+  unsigned p;
----------------
aemerson wrote:
> This function currently copied but can be shared with the DAG, but I'm waiting for D98200 to land so we can put it in a common utils file.
Do you mean D110747?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4536
+  if (!matchUnaryPredicate(MRI, RHS, BuildUDIVPattern))
+    return nullptr;
+
----------------
Can this ever happen? If not, just assert it returns true? Is that what the comment in applyUDivByConst is referring to?


================
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())
----------------
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?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:4585
+  auto *RHSDef = MRI.getVRegDef(RHS);
+  auto Cst = isConstantOrConstantVector(*RHSDef, MRI);
+  if (!Cst)
----------------
Fold this into the "if".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110890