[RewriteStatepointsForGC] Exclude constant values from being considered live at a safepoint

[reames]

Summary

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.

[reames]

Upload the diff again to force an email after adding llvm-commits.

Full description copied by hand in case it doesn't get resent.

[RewriteStatepointsForGC] Exclude constant values from being considered live at a safepoint

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.

[sanjoy]

LGTM.

llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

if (Instruction *inst = dyn_cast<Instruction>(def)) {
         store->insertBefore(normalDest->getFirstNonPHI());
       } else {
         assert(!inst->isTerminator() &&
                "The only TerminatorInst that can produce a value is "
                "InvokeInst which is handled above.");
         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));
     }
   }
 
   assert(PromotableAllocas.size() == live.size() &&
          "we must have the same allocas with lives");
   if (!PromotableAllocas.empty()) {
     // apply mem2reg to promote alloca to SSA

 }
 
 // liveness computation via standard dataflow
 // -------------------------------------------------------------------
 
 // 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,
                                 BasicBlock::reverse_iterator rend,
                                 DenseSet<Value *> &LiveTmp) {
 
   for (BasicBlock::reverse_iterator ritr = rbegin; ritr != rend; ritr++) {
     Instruction *I = &*ritr;
 
     // KILL/Def - Remove this definition from LiveIn
     LiveTmp.erase(I);
 
     // Don't consider *uses* in PHI nodes, we handle their contribution to
     // predecessor blocks when we seed the LiveOut sets
     if (isa<PHINode>(I))
       continue;
 
     // USE - Add to the LiveIn set for this instruction
     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);
       }
     }
   }
 }
 
 static void computeLiveOutSeed(BasicBlock *BB, DenseSet<Value *> &LiveTmp) {
 
   for (BasicBlock *Succ : successors(BB)) {
     const BasicBlock::iterator E(Succ->getFirstNonPHI());
     for (BasicBlock::iterator I = Succ->begin(); I != E; I++) {
       PHINode *Phi = cast<PHINode>(&*I);
       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);
       }
     }
   }
 }
 
 static DenseSet<Value *> computeKillSet(BasicBlock *BB) {
   DenseSet<Value *> KillSet;

llvm/trunk/test/Transforms/RewriteStatepointsForGC/constants.ll

+; 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
+}
+
+
+