[PATCH] D36581: [DAG] Fix Node Replacement in PromoteIntBinOp

[Nirav Dave via Phabricator via llvm-commits]

niravd added a comment.

In https://reviews.llvm.org/D36581#838609, @efriedma wrote:

> This is why I hate DAGCombine; you have to constantly worry about the CSE map invalidating pointers you care about, and I'm never confident I'm handling it correctly.
>
> Could we simplify this code using makeEquivalentMemoryOrdering to insert the loads into the right spot in the DAG?  Or does that introduce its own RAUW problems?


No, this is a close but unrelated problem. This has been latent for a long while. I'm a little surprised that it required https://reviews.llvm.org/rL297695 to expose this. FWIW I've found that the way to deal with CSE invalidation from multiple replacements is to calculate which nodes need to be replaced upfront (either trivially or checking if there's more than one use) and replacing user nodes before their operands. Clunkier than I'd like, but straightforward. I also have a patch that more aggressively causes invalidation-based assertions where latent bugs may lie; I think it'll catch the lion share of the latent bugs.


https://reviews.llvm.org/D36581