[PATCH] D154307: [InstructionSimplify] Avoid simplifying ICmp without parent

Mon Jul 3 05:56:32 PDT 2023

fhahn added a comment.

In D154307#4467234 <https://reviews.llvm.org/D154307#4467234>, @nikic wrote:

> I believe this is related to the "phi of op" feature in NewGVN: https://github.com/llvm/llvm-project/blob/6954cb54252b50df95eea05c513e739b3ad2c8a0/llvm/lib/Transforms/Scalar/NewGVN.cpp#L2767 These cloned instructions won't have a parent.
>
> I'm not entirely sure what to do here. I don't think that this fix is sufficient: If we need support for instructions without parent for NewGVN, then we need to revert @arsenm's change entirely, not just this part. Alternatively we need a different way to handle this on the NewGVN side. It may be possible to insert the instructions, as long as we are sure to later remove them again. I'm not sure if this would cause problems with NewGVNs design though, which is not supposed to do IR modifications during the analysis phase.

NewGVN makes use of instructions without parents in multiple places; I'm surprised that there isn't more fallout from this. I am not sure if inserting the instructions in the existing IR would work well, but inserting them in a temporary block may work as workaround.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154307