[llvm] r305234 - [RS4GC] Drop invalid metadata after pointers are relocated

Anna Thomas via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 12 14:26:53 PDT 2017


Author: annat
Date: Mon Jun 12 16:26:53 2017
New Revision: 305234

URL: http://llvm.org/viewvc/llvm-project?rev=305234&view=rev
Log:
[RS4GC] Drop invalid metadata after pointers are relocated

Summary:
After RS4GC, we should drop metadata that is no longer valid. These metadata
is used by optimizations scheduled after RS4GC, and can cause a miscompile.
One such metadata is invariant.load which is used by LICM sinking transform.
After rewriting statepoints, the address of a load maybe relocated. With
invariant.load metadata on a load instruction, LICM sinking assumes the
loaded value (from a dererenceable address) to be invariant, and
rematerializes the load operand and the load at the exit block.
This transforms the IR to have an unrelocated use of the
address after a statepoint, which is incorrect.
Other metadata we conservatively remove are related to
dereferenceability and noalias metadata.

This patch drops such metadata on store and load instructions after
rewriting statepoints.

Reviewers: reames, sanjoy, apilipenko

Reviewed by: reames

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D33756

Added:
    llvm/trunk/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp?rev=305234&r1=305233&r2=305234&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp Mon Jun 12 16:26:53 2017
@@ -89,10 +89,10 @@ struct RewriteStatepointsForGC : public
       Changed |= runOnFunction(F);
 
     if (Changed) {
-      // stripNonValidAttributes asserts that shouldRewriteStatepointsIn
+      // stripNonValidAttributesAndMetadata asserts that shouldRewriteStatepointsIn
       // returns true for at least one function in the module.  Since at least
       // one function changed, we know that the precondition is satisfied.
-      stripNonValidAttributes(M);
+      stripNonValidAttributesAndMetadata(M);
     }
 
     return Changed;
@@ -105,20 +105,24 @@ struct RewriteStatepointsForGC : public
     AU.addRequired<TargetTransformInfoWrapperPass>();
   }
 
-  /// The IR fed into RewriteStatepointsForGC may have had attributes implying
-  /// dereferenceability that are no longer valid/correct after
-  /// RewriteStatepointsForGC has run.  This is because semantically, after
+  /// The IR fed into RewriteStatepointsForGC may have had attributes and
+  /// metadata implying dereferenceability that are no longer valid/correct after
+  /// RewriteStatepointsForGC has run. This is because semantically, after
   /// RewriteStatepointsForGC runs, all calls to gc.statepoint "free" the entire
-  /// heap.  stripNonValidAttributes (conservatively) restores correctness
-  /// by erasing all attributes in the module that externally imply
-  /// dereferenceability.
-  /// Similar reasoning also applies to the noalias attributes. gc.statepoint
-  /// can touch the entire heap including noalias objects.
-  void stripNonValidAttributes(Module &M);
+  /// heap. stripNonValidAttributesAndMetadata (conservatively) restores
+  /// correctness by erasing all attributes in the module that externally imply
+  /// dereferenceability. Similar reasoning also applies to the noalias
+  /// attributes and metadata. gc.statepoint can touch the entire heap including
+  /// noalias objects.
+  void stripNonValidAttributesAndMetadata(Module &M);
 
-  // Helpers for stripNonValidAttributes
-  void stripNonValidAttributesFromBody(Function &F);
+  // Helpers for stripNonValidAttributesAndMetadata
+  void stripNonValidAttributesAndMetadataFromBody(Function &F);
   void stripNonValidAttributesFromPrototype(Function &F);
+  // Certain metadata on instructions are invalid after running RS4GC.
+  // Optimizations that run after RS4GC can incorrectly use this metadata to
+  // optimize functions. We drop such metadata on the instruction.
+  void stripInvalidMetadataFromInstruction(Instruction &I);
 };
 } // namespace
 
@@ -2306,13 +2310,44 @@ RewriteStatepointsForGC::stripNonValidAt
     RemoveNonValidAttrAtIndex(Ctx, F, AttributeList::ReturnIndex);
 }
 
-void RewriteStatepointsForGC::stripNonValidAttributesFromBody(Function &F) {
+void RewriteStatepointsForGC::stripInvalidMetadataFromInstruction(Instruction &I) {
+
+  if (!isa<LoadInst>(I) && !isa<StoreInst>(I))
+    return;
+  // These are the attributes that are still valid on loads and stores after
+  // RS4GC.
+  // The metadata implying dereferenceability and noalias are (conservatively)
+  // dropped.  This is because semantically, after RewriteStatepointsForGC runs,
+  // all calls to gc.statepoint "free" the entire heap. Also, gc.statepoint can
+  // touch the entire heap including noalias objects. Note: The reasoning is
+  // same as stripping the dereferenceability and noalias attributes that are
+  // analogous to the metadata counterparts.
+  // We also drop the invariant.load metadata on the load because that metadata
+  // implies the address operand to the load points to memory that is never
+  // changed once it became dereferenceable. This is no longer true after RS4GC.
+  // Similar reasoning applies to invariant.group metadata, which applies to
+  // loads within a group.
+  unsigned ValidMetadataAfterRS4GC[] = {LLVMContext::MD_tbaa,
+                         LLVMContext::MD_range,
+                         LLVMContext::MD_alias_scope,
+                         LLVMContext::MD_nontemporal,
+                         LLVMContext::MD_nonnull,
+                         LLVMContext::MD_align,
+                         LLVMContext::MD_type};
+
+  // Drops all metadata on the instruction other than ValidMetadataAfterRS4GC.
+  I.dropUnknownNonDebugMetadata(ValidMetadataAfterRS4GC);
+
+}
+
+void RewriteStatepointsForGC::stripNonValidAttributesAndMetadataFromBody(Function &F) {
   if (F.empty())
     return;
 
   LLVMContext &Ctx = F.getContext();
   MDBuilder Builder(Ctx);
 
+
   for (Instruction &I : instructions(F)) {
     if (const MDNode *MD = I.getMetadata(LLVMContext::MD_tbaa)) {
       assert(MD->getNumOperands() < 5 && "unrecognized metadata shape!");
@@ -2333,6 +2368,8 @@ void RewriteStatepointsForGC::stripNonVa
       I.setMetadata(LLVMContext::MD_tbaa, MutableTBAA);
     }
 
+    stripInvalidMetadataFromInstruction(I);
+
     if (CallSite CS = CallSite(&I)) {
       for (int i = 0, e = CS.arg_size(); i != e; i++)
         if (isa<PointerType>(CS.getArgument(i)->getType()))
@@ -2357,7 +2394,7 @@ static bool shouldRewriteStatepointsIn(F
     return false;
 }
 
-void RewriteStatepointsForGC::stripNonValidAttributes(Module &M) {
+void RewriteStatepointsForGC::stripNonValidAttributesAndMetadata(Module &M) {
 #ifndef NDEBUG
   assert(any_of(M, shouldRewriteStatepointsIn) && "precondition!");
 #endif
@@ -2366,7 +2403,7 @@ void RewriteStatepointsForGC::stripNonVa
     stripNonValidAttributesFromPrototype(F);
 
   for (Function &F : M)
-    stripNonValidAttributesFromBody(F);
+    stripNonValidAttributesAndMetadataFromBody(F);
 }
 
 bool RewriteStatepointsForGC::runOnFunction(Function &F) {

Added: llvm/trunk/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll?rev=305234&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll (added)
+++ llvm/trunk/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll Mon Jun 12 16:26:53 2017
@@ -0,0 +1,92 @@
+; RUN: opt -S -rewrite-statepoints-for-gc < %s | FileCheck %s
+
+; This test checks that metadata that's invalid after RS4GC is dropped. 
+; We can miscompile if optimizations scheduled after RS4GC uses the
+; metadata that's infact invalid.
+
+declare void @bar()
+
+declare void @baz(i32)
+; Confirm that loadedval instruction does not contain invariant.load metadata.
+; but contains the range metadata.
+; Since loadedval is not marked invariant, it will prevent incorrectly sinking
+; %loadedval in LICM and avoid creation of an unrelocated use of %baseaddr.
+define void @test_invariant_load() gc "statepoint-example" {
+; CHECK-LABEL: @test_invariant_load
+; CHECK: %loadedval = load i32, i32 addrspace(1)* %baseaddr, align 8, !range !0
+bb:
+  br label %outerloopHdr
+
+outerloopHdr:                                              ; preds = %bb6, %bb
+  %baseaddr = phi i32 addrspace(1)* [ undef, %bb ], [ %tmp4, %bb6 ]
+; LICM may sink this load to exit block after RS4GC because it's tagged invariant.
+  %loadedval = load i32, i32 addrspace(1)* %baseaddr, align 8, !range !0, !invariant.load !1
+  br label %innerloopHdr
+
+innerloopHdr:                                              ; preds = %innerlooplatch, %outerloopHdr
+  %tmp4 = phi i32 addrspace(1)* [ %baseaddr, %outerloopHdr ], [ %gep, %innerlooplatch ]
+  br label %innermostloophdr
+
+innermostloophdr:                                              ; preds = %bb6, %innerloopHdr
+  br i1 undef, label %exitblock, label %bb6
+
+bb6:                                              ; preds = %innermostloophdr
+  switch i32 undef, label %innermostloophdr [
+    i32 0, label %outerloopHdr
+    i32 1, label %innerlooplatch
+  ]
+
+innerlooplatch:                                              ; preds = %bb6
+  call void @bar()
+  %gep = getelementptr inbounds i32, i32 addrspace(1)* %tmp4, i64 8
+  br label %innerloopHdr
+
+exitblock:                                             ; preds = %innermostloophdr
+  %tmp13 = add i32 42, %loadedval
+  call void @baz(i32 %tmp13)
+  unreachable
+}
+
+; drop the noalias metadata.
+define void @test_noalias(i32 %x, i32 addrspace(1)* %p, i32 addrspace(1)* %q) gc "statepoint-example" {
+; CHECK-LABEL: test_noalias
+; CHECK: %y = load i32, i32 addrspace(1)* %q, align 16
+; CHECK: gc.statepoint
+; CHECK: %p.relocated
+; CHECK-NEXT: %p.relocated.casted = bitcast i8 addrspace(1)* %p.relocated to i32 addrspace(1)*
+; CHECK-NEXT: store i32 %x, i32 addrspace(1)* %p.relocated.casted, align 16
+entry:
+  %y = load i32, i32 addrspace(1)* %q, align 16, !noalias !3
+  call void @baz(i32 %x)
+  store i32 %x, i32 addrspace(1)* %p, align 16, !noalias !4
+  ret void
+}
+
+; drop the dereferenceable metadata
+define void @test_dereferenceable(i32 addrspace(1)* addrspace(1)* %p, i32 %x, i32 addrspace(1)* %q) gc "statepoint-example" {
+; CHECK-LABEL: test_dereferenceable
+; CHECK: %v1 = load i32 addrspace(1)*, i32 addrspace(1)* addrspace(1)* %p
+; CHECK-NEXT: %v2 = load i32, i32 addrspace(1)* %v1
+; CHECK: gc.statepoint
+  %v1 = load i32 addrspace(1)*, i32 addrspace(1)* addrspace(1)* %p, !dereferenceable !5
+  %v2 = load i32, i32 addrspace(1)* %v1
+  call void @baz(i32 %x)
+  store i32 %v2, i32 addrspace(1)* %q, align 16
+  ret void
+}
+
+declare token @llvm.experimental.gc.statepoint.p0f_isVoidi32f(i64, i32, void (i32)*, i32, i32, ...)
+
+; Function Attrs: nounwind readonly
+declare i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token, i32, i32) #0
+
+declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)
+
+attributes #0 = { nounwind readonly }
+
+!0 = !{i32 0, i32 2147483647}
+!1 = !{}
+!2 = !{i32 10, i32 1}
+!3 = !{!3}
+!4 = !{!4}
+!5 = !{i64 8}




More information about the llvm-commits mailing list