[PATCH] D97837: [InstCombine] Remove gc.relocate duplicates

[Serguei Katkov via Phabricator via llvm-commits]

skatkov abandoned this revision.
skatkov added a comment.

In D97837#2614207 <https://reviews.llvm.org/D97837#2614207>, @reames wrote:

> In D97837#2612986 <https://reviews.llvm.org/D97837#2612986>, @skatkov wrote:
>
>> In D97837#2608208 <https://reviews.llvm.org/D97837#2608208>, @reames wrote:
>>
>>> Posted an enhancement for phi handling in GVN: https://reviews.llvm.org/D98080
>>>
>>> I'd be curious to know if that's enough for the original test case.
>>
>> D97974 <https://reviews.llvm.org/D97974> + D98080 <https://reviews.llvm.org/D98080> does not produce the best result.
>> It does not remove dup gc.relocate since some point by some reason.
>> The following pipeline produces the best result:
>> -passes=rewrite-statepoints-for-gc,gvn,instcombine,gvn,instcombine,gvn,instcombine,gvn,instcombine.
>
> Separately from the RS4GC work, would you mind reducing and sharing a test case for this?  I'd like to understand why GVN isn't getting this case.  >From what you wrote, it definitely *should*.

Hi Philip, I have a trouble to share the original test but after careful merging of all current GVN patches to downstream I ensured that gvn + instcombine gives the same simplification as I saw best one.
So I can confirm that current GVN can handle simplification in a one pass.

With that I abandon this patch and will rely on GVN. However simplification in RS4GC still makes sense to me.


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

https://reviews.llvm.org/D97837