[PATCH] D51169: [NewGVN] Mark function as changed if we erase instructions.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In https://reviews.llvm.org/D51169#1225675, @bjope wrote:

> An alternative could perhaps be to do something like this
>
>   Changed |= !InstructionsToErase.empty();
>
>
> before the loop (assuming that if we got something in InstructionsToErase then we also have done some modifications).
>  But I think your solution (setting Changed whenever we do a change) is easier to understand.
>
> Thanks for the patch. LGTM!


Thanks! I just had another look and I think even though we have 2 ifs in the loop over InstructionsToErase, we should always do modifications if it is not empty. In particular, if an instruction to erase does not have a parent any more, that would mean the parent BB has been deleted earlier, so we already did modifications. I think I'll go with your suggestion!


https://reviews.llvm.org/D51169