[PATCH] D118591: [Function Specialisation] Fix use after free

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added a comment.

In D118591#3286671 <https://reviews.llvm.org/D118591#3286671>, @SjoerdMeijer wrote:

> Agreed, lazily removing the instructions in this way is what I was expecting.
>
> But this is a bit odd, a.k.a. not what I was expecting:
>
>> None of the existing tests actually covers this code path I am afraid.
>
> Do you mean this:
>
>   if (I->isSafeToRemove()) {
>      I->eraseFromParent();
>      Solver.removeLatticeValueFor(I);
>    }
>
> Only triggers in the reproducer attached to the bugreport and not in the regression tests? I think we should first try hard to get a test for that, which I hope is doable. Otherwise, we might as well remove this code and rely on later clean up passes to delete dead code which I think is a valid strategy and alternative if it makes things really complicated here. But again, better would be to see if we can trigger this and a test for it.

I was wrong when I said we are not testing the removal of dead instructions. I was looking at the tests under the original commit D93838 <https://reviews.llvm.org/rGc4a0969b9c14acc795ae9e841b8289c3d36220b1>, but we have actually added more tests since. For example `function-specialization-constant-expression.ll` is exercising this code path.


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

https://reviews.llvm.org/D118591