[PATCH] D154678: [InstCombine] Fold IEEE `fmul`/`fdiv` by Pow2 to `add`/`sub` of exp

[Nikita Popov via Phabricator via llvm-commits]

nikic added a reviewer: efriedma.
nikic added a comment.

Sounds plausible, but I'll leave this to the FP experts...



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:573
+  // Only do IEEE floats. Also skip scalable vecs.
+  if (!FPTy->getScalarType()->isIEEELikeFPTy() || FPTy->isScalableTy())
+    return nullptr;
----------------
Why does this exclude scalable vectors?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:625
+               isKnownNonZero(V, Q.DL, /*Depth*/ 0, Q.AC, Q.CxtI, Q.DT),
+               /*DoFold*/ true);
+  if (Log2 == nullptr)
----------------
Should call this with DoFold = false first, otherwise it'll create dead instructions. They'll get removed, but I think this will lead to an infinite combine loop.

Please add a test where this fails to fold after some sub-part has folded.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:633-639
+  if (FPTy->isVectorTy()) {
+    auto *FVTy = dyn_cast<FixedVectorType>(FPTy);
+    assert(FVTy &&
+           "We should have already made sure this is not a scalable type!");
+    IntTy =
+        VectorType::get(IntTy, ElementCount::getFixed(FVTy->getNumElements()));
+  }
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154678