[PATCH] D33756: [RS4GC] Drop invalid metadata after pointers are relocated

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2351
+    // metadata that's invalid after RS4GC.
+    if (isa<LoadInst>(I) && I.getMetadata(LLVMContext::MD_invariant_load))
+      I.dropUnknownNonDebugMetadata(KnownIDs);
----------------
anna wrote:
> reames wrote:
> > I think that a better factoring on this would be to factor out something from RemoveNonValidAttrAtIndex which works on a load, call it on a load, then add !invariant.load to that list.  This is not specific to invariant load.  Really, we'd just completely failed to think about loads in implementing this.
> I don't think we can factor out anything from `RemoveNonValidAttrAtIndex`. We are trying to remove metadata and not attributes. Also, there is no way to specify something like `I.removeMetadata(invariant_load)`, and such a function should *not* be added in Instruction class. The reason being we need to keep auditing code for correctness to keep track of all new metadata that gets added to langref.  The `dropUnknownNonDebugMetadata(knownIDs)` works *correctly* without any changes even when new metadata gets added.
> 
> Also, `I.dropUnknownNonDebugMetadata` updates private members in Instruction once no metadata is left behind in that instruction. 
> 
> I can introduce a function like `DropInvalidMetadataOnInstruction` and call it on a load instruction. Does that work?
> 
> 
Talked to Philip offline, and he agreed with my analysis (since we are dealing with metadata and not attributes). 

I'm going to update the comments and code to make this distinction clearer. 


https://reviews.llvm.org/D33756