[PATCH] D25096: [RS4GC] New pass to remove gc.relocates added by RS4GC
Anna Thomas via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 21 08:55:47 PDT 2016
anna marked an inline comment as done.
anna added inline comments.
Comment at: lib/Transforms/Utils/RemoveGCRelocates.cpp:49
+ // Nothing to do for declarations.
+ if (F.isDeclaration() || F.empty())
+ return false;
> I didn't know we could have empty non-declarations.
Actually, I think there cannot be empty non-declarations after all optimizations are done.
Either way the pass will not do anything for empty functions. Removed that check.
Comment at: lib/Transforms/Utils/RemoveGCRelocates.cpp:70
+ Instruction *ToBeReplaced = GCRel;
+ // All gc_relocates are i8 addrspace(1)* typed, and may be bitcasted to
> I wouldn't bother with this. I'd just insert a bitcast of the original to `i8 addrspace(1)*`, RAUW with that. We can always run `-instcombine` later to clean up the IR quickly.
> At the very least, I'd do this ^ in place of the `continue` down below:
> if (GCRel->hasOneUse() && isa<BitCastInst>(GCRel->user_back()))
> ToBeReplaced = GCRel->user_back();
> // Create new bitcast and RAUW with that.
Yes, the second idea was what I was thinking of originally, and mentioned in comment below.
I like your first idea better, the code is cleaner, and instcombine would get rid of unnecessary bitcasts (if any).
More information about the llvm-commits