[RewriteStatepointsForGC] Exclude constant values from being considered live at a...
authorPhilip Reames <listmail@philipreames.com>
Sun, 26 Apr 2015 19:48:03 +0000 (19:48 +0000)
committerPhilip Reames <listmail@philipreames.com>
Sun, 26 Apr 2015 19:48:03 +0000 (19:48 +0000)
There can be various constant pointers in the IR which do not get relocated at a safepoint. One example is the address of a global variable. Another example is a pointer created via inttoptr. Note that the optimizer itself likes to create such inttoptrs when locally propagating constants through dynamically dead code.

To deal with this, we need to exclude uses of constants from contributing to the liveness of a safepoint which might reach that use. At some later date, it might be worth exploring what could be done to support the relocation of various special types of "constants", but that's future work.

Differential Revision: http://reviews.llvm.org/D9236

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@235821 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
test/Transforms/RewriteStatepointsForGC/constants.ll [new file with mode: 0644]

index ba48e0a74b0b510acac0f3c28664b7caed2a039f..3da3559ca1f41fe6e68f0a8eb037869457ab5544 100644 (file)
@@ -1504,9 +1504,7 @@ static void relocationViaAlloca(
         store->insertAfter(inst);
       }
     } else {
-      assert((isa<Argument>(def) || isa<GlobalVariable>(def) ||
-              isa<ConstantPointerNull>(def)) &&
-             "Must be argument or global");
+      assert(isa<Argument>(def));
       store->insertAfter(cast<Instruction>(alloca));
     }
   }
@@ -1958,11 +1956,6 @@ bool RewriteStatepointsForGC::runOnFunction(Function &F) {
 // TODO: Consider using bitvectors for liveness, the set of potentially
 // interesting values should be small and easy to pre-compute.
 
-/// Is this value a constant consisting of entirely null values?
-static bool isConstantNull(Value *V) {
-  return isa<Constant>(V) && cast<Constant>(V)->isNullValue();
-}
-
 /// Compute the live-in set for the location rbegin starting from
 /// the live-out set of the basic block
 static void computeLiveInValues(BasicBlock::reverse_iterator rbegin,
@@ -1984,9 +1977,17 @@ static void computeLiveInValues(BasicBlock::reverse_iterator rbegin,
     for (Value *V : I->operands()) {
       assert(!isUnhandledGCPointerType(V->getType()) &&
              "support for FCA unimplemented");
-      if (isHandledGCPointerType(V->getType()) && !isConstantNull(V) &&
-          !isa<UndefValue>(V)) {
-        // The choice to exclude null and undef is arbitrary here.  Reconsider?
+      if (isHandledGCPointerType(V->getType()) && !isa<Constant>(V)) {
+        // The choice to exclude all things constant here is slightly subtle.
+        // There are two idependent reasons:
+        // - We assume that things which are constant (from LLVM's definition)
+        // do not move at runtime.  For example, the address of a global
+        // variable is fixed, even though it's contents may not be.
+        // - Second, we can't disallow arbitrary inttoptr constants even
+        // if the language frontend does.  Optimization passes are free to
+        // locally exploit facts without respect to global reachability.  This
+        // can create sections of code which are dynamically unreachable and
+        // contain just about anything.  (see constants.ll in tests)
         LiveTmp.insert(V);
       }
     }
@@ -2002,9 +2003,7 @@ static void computeLiveOutSeed(BasicBlock *BB, DenseSet<Value *> &LiveTmp) {
       Value *V = Phi->getIncomingValueForBlock(BB);
       assert(!isUnhandledGCPointerType(V->getType()) &&
              "support for FCA unimplemented");
-      if (isHandledGCPointerType(V->getType()) && !isConstantNull(V) &&
-          !isa<UndefValue>(V)) {
-        // The choice to exclude null and undef is arbitrary here.  Reconsider?
+      if (isHandledGCPointerType(V->getType()) && !isa<Constant>(V)) {
         LiveTmp.insert(V);
       }
     }
diff --git a/test/Transforms/RewriteStatepointsForGC/constants.ll b/test/Transforms/RewriteStatepointsForGC/constants.ll
new file mode 100644 (file)
index 0000000..235e314
--- /dev/null
@@ -0,0 +1,61 @@
+; RUN: opt -S -rewrite-statepoints-for-gc %s | FileCheck %s
+
+declare void @foo()
+declare i32 @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()*, i32, i32, ...)
+
+; constants don't get relocated.
+define i8 @test() gc "statepoint-example" {
+; CHECK-LABEL: @test
+; CHECK: gc.statepoint
+; CHECK-NEXT: load i8, i8 addrspace(1)* inttoptr (i64 15 to i8 addrspace(1)*)
+entry:
+  call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
+  %res = load i8, i8 addrspace(1)* inttoptr (i64 15 to i8 addrspace(1)*)
+  ret i8 %res
+}
+
+
+; Mostly just here to show reasonable code test can come from.  
+define i8 @test2(i8 addrspace(1)* %p) gc "statepoint-example" {
+; CHECK-LABEL: @test2
+; CHECK: gc.statepoint
+; CHECK-NEXT: gc.relocate
+; CHECK-NEXT: icmp
+entry:
+  call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
+  %cmp = icmp eq i8 addrspace(1)* %p, null
+  br i1 %cmp, label %taken, label %not_taken
+
+taken:
+  ret i8 0
+
+not_taken:
+  %cmp2 = icmp ne i8 addrspace(1)* %p, null
+  br i1 %cmp2, label %taken, label %dead
+
+dead:
+  ; We see that dead can't be reached, but the optimizer might not.  It's 
+  ; completely legal for it to exploit the fact that if dead executed, %p 
+  ; would have to equal null.  This can produce intermediate states which 
+  ; look like that of test above, even if arbitrary constant addresses aren't
+  ; legal in the source language
+  %addr = getelementptr i8, i8 addrspace(1)* %p, i32 15
+  %res = load i8, i8addrspace(1)* %addr
+  ret i8 %res
+}
+
+@G = addrspace(1) global i8 5
+
+; Globals don't move and thus don't get relocated
+define i8 @test3(i1 %always_true) gc "statepoint-example" {
+; CHECK-LABEL: @test3
+; CHECK: gc.statepoint
+; CHECK-NEXT: load i8, i8 addrspace(1)* @G
+entry:
+  call i32 (void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(void ()* @foo, i32 0, i32 0, i32 0)
+  %res = load i8, i8 addrspace(1)* @G, align 1
+  ret i8 %res
+}
+
+
+