Code simplification and style cleanup

All the removed assertions are either implied locally by the assert at the top of the function or properties of the verifier.

llvm-svn: 233358

llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

@@ -313,51 +313,29 @@ static Value *findBaseDefiningValue(Value *I) {
          "Illegal to ask for the base pointer of a non-pointer type");
 
   // There are instructions which can never return gc pointer values.  Sanity
-  // check
+  // check that this is actually true.
-  // that this is actually true.
   assert(!isa<InsertElementInst>(I) && !isa<ExtractElementInst>(I) &&
          !isa<ShuffleVectorInst>(I) && "Vector types are not gc pointers");
-  assert((!isa<Instruction>(I) || isa<InvokeInst>(I) ||
-          !cast<Instruction>(I)->isTerminator()) &&
-         "With the exception of invoke terminators don't define values");
-  assert(!isa<StoreInst>(I) && !isa<FenceInst>(I) &&
-         "Can't be definitions to start with");
-  assert(!isa<ICmpInst>(I) && !isa<FCmpInst>(I) &&
-         "Comparisons don't give ops");
-  // There's a bunch of instructions which just don't make sense to apply to
-  // a pointer.  The only valid reason for this would be pointer bit
-  // twiddling which we're just not going to support.
-  assert((!isa<Instruction>(I) || !cast<Instruction>(I)->isBinaryOp()) &&
-         "Binary ops on pointer values are meaningless.  Unless your "
-         "bit-twiddling which we don't support");
 
-  if (Argument *Arg = dyn_cast<Argument>(I)) {
+  if (isa<Argument>(I))
     // An incoming argument to the function is a base pointer
     // We should have never reached here if this argument isn't an gc value
-    assert(Arg->getType()->isPointerTy() &&
+    return I;
-           "Base for pointer must be another pointer");
-    return Arg;
-  }
 
-  if (GlobalVariable *global = dyn_cast<GlobalVariable>(I)) {
+  if (isa<GlobalVariable>(I))
     // base case
-    assert(global->getType()->isPointerTy() &&
+    return I;
-           "Base for pointer must be another pointer");
-    return global;
-  }
 
   // inlining could possibly introduce phi node that contains
   // undef if callee has multiple returns
-  if (UndefValue *undef = dyn_cast<UndefValue>(I)) {
+  if (isa<UndefValue>(I))
-    assert(undef->getType()->isPointerTy() &&
+    // utterly meaningless, but useful for dealing with
-           "Base for pointer must be another pointer");
+    // partially optimized code.
-    return undef; // utterly meaningless, but useful for dealing with
+    return I; 
-                  // partially optimized code.
-  }
 
   // Due to inheritance, this must be _after_ the global variable and undef
   // checks
-  if (Constant *con = dyn_cast<Constant>(I)) {
+  if (Constant *Con = dyn_cast<Constant>(I)) {
     assert(!isa<GlobalVariable>(I) && !isa<UndefValue>(I) &&
            "order of checks wrong!");
     // Note: Finding a constant base for something marked for relocation
@@ -368,49 +346,31 @@ static Value *findBaseDefiningValue(Value *I) {
     // off a potentially null value and have proven it null.  We also use
     // null pointers in dead paths of relocation phis (which we might later
     // want to find a base pointer for).
-    assert(con->getType()->isPointerTy() &&
+    assert(Con->getType()->isPointerTy() &&
            "Base for pointer must be another pointer");
-    assert(con->isNullValue() && "null is the only case which makes sense");
+    assert(Con->isNullValue() && "null is the only case which makes sense");
-    return con;
+    return Con;
   }
 
   if (CastInst *CI = dyn_cast<CastInst>(I)) {
-    Value *def = CI->stripPointerCasts();
+    Value *Def = CI->stripPointerCasts();
-    assert(def->getType()->isPointerTy() &&
-           "Base for pointer must be another pointer");
     // If we find a cast instruction here, it means we've found a cast which is
     // not simply a pointer cast (i.e. an inttoptr).  We don't know how to
     // handle int->ptr conversion.
-    assert(!isa<CastInst>(def) && "shouldn't find another cast here");
+    assert(!isa<CastInst>(Def) && "shouldn't find another cast here");
-    return findBaseDefiningValue(def);
+    return findBaseDefiningValue(Def);
   }
 
-  if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
+  if (isa<LoadInst>(I))
-    if (LI->getType()->isPointerTy()) {
+    return I; // The value loaded is an gc base itself
-      Value *Op = LI->getOperand(0);
-      (void)Op;
-      // Has to be a pointer to an gc object, or possibly an array of such?
-      assert(Op->getType()->isPointerTy());
-      return LI; // The value loaded is an gc base itself
-    }
-  }
-  if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(I)) {
-    Value *Op = GEP->getOperand(0);
-    if (Op->getType()->isPointerTy()) {
-      return findBaseDefiningValue(Op); // The base of this GEP is the base
-    }
-  }
 
-  if (AllocaInst *alloc = dyn_cast<AllocaInst>(I)) {
+  if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(I))
-    // An alloca represents a conceptual stack slot.  It's the slot itself
+    // The base of this GEP is the base
-    // that the GC needs to know about, not the value in the slot.
+    return findBaseDefiningValue(GEP->getPointerOperand());
-    assert(alloc->getType()->isPointerTy() &&
-           "Base for pointer must be another pointer");
-    return alloc;
-  }
 
   if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
     switch (II->getIntrinsicID()) {
+    case Intrinsic::experimental_gc_result_ptr:
     default:
       // fall through to general call handling
       break;
@@ -418,11 +378,6 @@ static Value *findBaseDefiningValue(Value *I) {
     case Intrinsic::experimental_gc_result_float:
     case Intrinsic::experimental_gc_result_int:
       llvm_unreachable("these don't produce pointers");
-    case Intrinsic::experimental_gc_result_ptr:
-      // This is just a special case of the CallInst check below to handle a
-      // statepoint with deopt args which hasn't been rewritten for GC yet.
-      // TODO: Assert that the statepoint isn't rewritten yet.
-      return II;
     case Intrinsic::experimental_gc_relocate: {
       // Rerunning safepoint insertion after safepoints are already
       // inserted is not supported.  It could probably be made to work,
@@ -440,41 +395,27 @@ static Value *findBaseDefiningValue(Value *I) {
   // We assume that functions in the source language only return base
   // pointers.  This should probably be generalized via attributes to support
   // both source language and internal functions.
-  if (CallInst *call = dyn_cast<CallInst>(I)) {
+  if (isa<CallInst>(I) || isa<InvokeInst>(I))
-    assert(call->getType()->isPointerTy() &&
+    return I;
-           "Base for pointer must be another pointer");
-    return call;
-  }
-  if (InvokeInst *invoke = dyn_cast<InvokeInst>(I)) {
-    assert(invoke->getType()->isPointerTy() &&
-           "Base for pointer must be another pointer");
-    return invoke;
-  }
 
   // I have absolutely no idea how to implement this part yet.  It's not
   // neccessarily hard, I just haven't really looked at it yet.
   assert(!isa<LandingPadInst>(I) && "Landing Pad is unimplemented");
 
-  if (AtomicCmpXchgInst *cas = dyn_cast<AtomicCmpXchgInst>(I)) {
+  if (isa<AtomicCmpXchgInst>(I))
     // A CAS is effectively a atomic store and load combined under a
     // predicate.  From the perspective of base pointers, we just treat it
-    // like a load.  We loaded a pointer from a address in memory, that value
+    // like a load.
-    // had better be a valid base pointer.
+    return I;
-    return cas->getPointerOperand();
+  
-  }
+  assert(!isa<AtomicRMWInst>(I) && "Xchg handled above, all others are "
-  if (AtomicRMWInst *atomic = dyn_cast<AtomicRMWInst>(I)) {
+         "binary ops which don't apply to pointers");
-    assert(AtomicRMWInst::Xchg == atomic->getOperation() &&
-           "All others are binary ops which don't apply to base pointers");
-    // semantically, a load, store pair.  Treat it the same as a standard load
-    return atomic->getPointerOperand();
-  }
 
   // The aggregate ops.  Aggregates can either be in the heap or on the
   // stack, but in either case, this is simply a field load.  As a result,
   // this is a defining definition of the base just like a load is.
-  if (ExtractValueInst *ev = dyn_cast<ExtractValueInst>(I)) {
+  if (isa<ExtractValueInst>(I))
-    return ev;
+    return I;
-  }
 
   // We should never see an insert vector since that would require we be
   // tracing back a struct value not a pointer value.
@@ -485,11 +426,9 @@ static Value *findBaseDefiningValue(Value *I) {
   // return a value which dynamically selects from amoung several base
   // derived pointers (each with it's own base potentially).  It's the job of
   // the caller to resolve these.
-  if (SelectInst *select = dyn_cast<SelectInst>(I)) {
+  assert((isa<SelectInst>(I) || isa<PHINode>(I)) && 
-    return select;
+         "missing instruction case in findBaseDefiningValing");
-  }
+  return I;
-
-  return cast<PHINode>(I);
 }
 
 /// Returns the base defining value for this value.