[PATCH] D94862: [InstCombine] Replace one-use select operand based on condition

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Thanks for the reproducer @hans. I have applied a fixed version of this patch in 21443381c00d9d5ddd6a73f2f839dc4872d79463 <https://reviews.llvm.org/rG21443381c00d9d5ddd6a73f2f839dc4872d79463>, which I believe should both fix the issue you saw, and a more general problem.

For the chromium issue, the problem is that we're replacing an operand of a phi node, but phi operands refer to values from their predecessor block, not the current block. As such an `%x` in a select and an `%x` in a phi operand may refer to different values and we can't replace them in the general case.

The more general problem is that we can only replace operands on speculatable instructions. If the instruction isn't speculatable, then replacing the operand might either cause an observable side-effect change, or cause UB.

Both of these issues are addressed by an additional isSafeToSpeculativelyExecute() check (phis are not considered speculatable).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94862