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

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

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

> Serguei,
>
> Your explanation of the pass ordering interaction makes a lot of sense, thank you for the nice explanation.  However, I think there's still something to the story missing.
>
> EarlyCSE already has dedicated support for this case.  Consider the lines:
>
>   // gc.relocate is 'special' call: its second and third operands are
>   // not real values, but indices into statepoint's argument list.
>   // Get values they point to.
>   if (const GCRelocateInst *GCR = dyn_cast<GCRelocateInst>(Inst))
>     return hash_combine(GCR->getOpcode(), GCR->getOperand(0),
>                         GCR->getBasePtr(), GCR->getDerivedPtr());
>
> What should happen here is that at each statepoint we canonicalize the gc.relocates (but leave the duplicate entries in the gc bundle of the statepoint) in a single pass over the IR.  We in fact have a test (test/Transforms/EarlyCSE/gc_relocate.ll) which seems to show exactly that happening.
>
> Is it possible that you don't have earlycse at the relevant point in your pass pipeline?
>
> I will note that I don't see an analogous change having been made to GVN.  So it's possible that's your problem.  (It would be a straight forward tweak in lookupOrAddCall.)

Thanks a lot! I neede to look into CSE yesterday to save your time, my bad. Indeed CSE is able to make simplification in one pass.
So to make full cleanup after RS4GC we need

1. InstCombine to eliminate redundant phi
2. EarlyCSE to clean-up the chain of gc.relocation
3. InstCombine again to clean-up gc bundle

InstCombine with this patch is able to do it by one pass but anyway if you think that CSE optimization is irrelevant in InstCombine I will abandon this patch.

Let me take a look into GVN whether with your patch GVN can do a full clean-up as in one pass...


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

https://reviews.llvm.org/D97837