[PATCH] D65204: [GVN] Also invalidate users of instructions replaced due to conditionals.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D65204#1600091 <https://reviews.llvm.org/D65204#1600091>, @efriedma wrote:

> Oh, okay, that makes more sense.
>
> > cache them using the translated pointer (%_tmp47)
>
> Does it really make sense to cache the load %_tmp58 using the pointer %_tmp47, as opposed to the pointer %_tmp57?  There isn't any dominance relationship between %_tmp47 and %_tmp58.


I think it makes sense in this use case, given we just want to find a single value to represent equivalent addresses and we are not using them to replace anything. But it requires careful cache invalidation and we get that wrong in a bunch of places.

We could ask for addresses that dominate the blocks we are translating through, but it would limit both the number of addresses we can translate and also increase the number of cache misses (6 existing GVN test cases fail when requiring dominating addresses). What do you think? Should we fix the issues with invalidation or restrict the caching/translating?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65204