[PATCH] D103316: Hoist llvm.assume into single predecessor if block otherwise empty

[Nikita Popov via Phabricator via llvm-commits]

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

> This kind of transformation should probably eventually reside in lib/Transforms/Utils/SimplifyCFG.cpp but for now it seemed easier and a smaller change to just leave it in CodeGenPrepare.

I don't think this transform makes a lot of sense in CGP, I'd prefer it to go directly to SimplifyCFG (after which we can drop the CGP code).

However, I'm not sure the whole concept of "hoisting assumes" even makes sense. I'm not sure if we have any passes that can reason about `assume(c1 || c2)` conditions. We have a lot that can reason about `assume(c1 && c2)` once it has been canonicalized to `assume(c1) assume(c2)`, but probably nothing that handles ors. The only case in which it could end up being useful is if at a later point, we can show that `c1` is actually `true`, which means we can fold it to just `assume(c2)`, at which point it becomes usable.

Another thing to consider is that assumes with operand bundles would require a more complex transform, to convert the operand bundle form back into a condition form.

I think we would be better off to just drop these assumes. The framing I'm thinking of here is that SimplifyCFG should generally be ignoring ephemeral values in transforms. So if normally SimplifyCFG would drop a block that contains no instructions, then it should also drop a block that only contains ephemeral values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103316