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

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

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.)


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

https://reviews.llvm.org/D97837