[PATCH] D141712: [GVN] Improve PRE on load instructions

[Sergei Kachkov via Phabricator via llvm-commits]

kachkov98 added a comment.

I have a question regarding code example: it looks like a case of load hoisting from SuccBB and basic block after splitting of (PredBB, LoadBB) critical edge. If so, the same result can be achieved by:

1. running SimplifyCFGOpt::HoistThenElseCodeToIf after GVN. Its implementation is very constrained (can hoist only first N exactly the same instructions from ThenBB and ElseBB, so very sensitive to instruction order)
2. Run GVNHoist pass after GVN -- the most general solution. Unfortunately it's disabled by default and placed before GVN in pass pipeline (although GVN exposes lots of hoisting opportunities, and its authors advise to run it before and after GVN-PRE).

Also there was an attempt to implement intermediate solution (hoist only instructions from 2 BBs to their common successors, but match them with value numbers): https://discourse.llvm.org/t/rfc-simple-gvn-hoist, so there is a chance that https://reviews.llvm.org/D110822 also covers the same problem.

In general, I'm concerned that this patch tries to combine 2 different transformations: load PRE and hoisting. It can omit critical edge splitting in some cases, but after hoisting we can also cleanup CFG.


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

https://reviews.llvm.org/D141712