[PATCH] D25096: [RS4GC] New pass to remove gc.relocates added by RS4GC

[Sanjoy Das via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added a comment.
This revision now requires changes to proceed.

I have some minor comments inline, but overall I'm not sure if a pass like this (solely useful for generating "debugging output") is okay to go upstream.

Do you mind starting a thread on llvm-dev discussing if this sort of thing is okay with the community?



> RemoveGCRelocates.cpp:1
> +//===- InstructionNamer.cpp - Give anonymous instructions names -----------===//
> +//

Fix the comment here.

> RemoveGCRelocates.cpp:41
> +
> +private:
> +  SmallVector<Instruction *, 20> GCRelocates;

Not sure if you need `private:` here.

> RemoveGCRelocates.cpp:69
> +    ConstantInt *LiveIdx = cast<ConstantInt>(GCRel->getOperand(2));
> +    Instruction *Statepoint = dyn_cast<Instruction>(GCRel->getOperand(0));
> +    if (!Statepoint)

Didn't you just check that `GCRel->getOperand(0)` is a statepoint?  I think this can be a `cast<>`

> RemoveGCRelocates.cpp:72
> +      continue;
> +    Value *OrigPtr = Statepoint->getOperand(LiveIdx->getZExtValue());
> +

Why not just use the `GCRelocateInst::getBasePtr` instruction?

https://reviews.llvm.org/D25096