[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;
sanjoy wrote:
> 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
sanjoy wrote:
> 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();
>       else
>         // 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 mailing list