[PATCH] D87555: [DivRemPairs] Add an initial case for hoisting to a common predecessor.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D87555#2269355 <https://reviews.llvm.org/D87555#2269355>, @lebedev.ri wrote:

> Can we use `DominatorTree::findNearestCommonDominator()`?
> If not, please add a negative test, where it's unsafe to do so.

The nearest common dominator could put the division on a path that it wasn't going to execute on right? Isn't that not just possibly unsafe but also possibly unprofitable?



================
Comment at: llvm/lib/Transforms/Scalar/DivRemPairs.cpp:241
       // neither of which dominates one another.
       // FIXME: We could hoist both ops to the common predecessor block?
+
----------------
lebedev.ri wrote:
> Remove FIXME?
I only handled one specific case. There are other cases that could be handled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87555