[PATCH] D143631: [LTO] Don't let InstCombine re-sink the vastly more expensive fdiv

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Depending on the LICM pass being run after InstCombine feels a bit like a game of cat and mouse, where the former hoists out the reciprocal and the latter restores things to the canonical form. I worry that guarding this ordering with just a test is a bit fragile.

There also isn't much precedent for changing InstCombine to //not// do the simplification, as I only spotted one cases in InstCombine (InstCombinerImpl::visitGEPOfGEP) where it looked at LoopInfo to see if something is loop invariant. Additionally, if having `1/x` outside the loop is the canonical form, it still relies on LICM being run first and for LoopInfo to be available in InstCombine (which in my understanding is optional, depending on the passes run before it).

So should this functionality perhaps be moved out of LICM into a more limited pass that is run just before SelectionDAG, or perhaps be made part of CodeGenPrepare?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143631