From c3bbff9f0c7405a7395f3d51e45b206bd2f47169 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Fri, 27 Mar 2015 05:34:44 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@233358 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Scalar/RewriteStatepointsForGC.cpp | 133 +++++------------- 1 file changed, 36 insertions(+), 97 deletions(-) diff --git a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp index f5d21fffda2..efbfcd7231e 100644 --- a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp +++ b/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 - // that this is actually true. + // check that this is actually true. assert(!isa(I) && !isa(I) && !isa(I) && "Vector types are not gc pointers"); - assert((!isa(I) || isa(I) || - !cast(I)->isTerminator()) && - "With the exception of invoke terminators don't define values"); - assert(!isa(I) && !isa(I) && - "Can't be definitions to start with"); - assert(!isa(I) && !isa(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(I) || !cast(I)->isBinaryOp()) && - "Binary ops on pointer values are meaningless. Unless your " - "bit-twiddling which we don't support"); - - if (Argument *Arg = dyn_cast(I)) { + + if (isa(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() && - "Base for pointer must be another pointer"); - return Arg; - } + return I; - if (GlobalVariable *global = dyn_cast(I)) { + if (isa(I)) // base case - assert(global->getType()->isPointerTy() && - "Base for pointer must be another pointer"); - return global; - } + return I; // inlining could possibly introduce phi node that contains // undef if callee has multiple returns - if (UndefValue *undef = dyn_cast(I)) { - assert(undef->getType()->isPointerTy() && - "Base for pointer must be another pointer"); - return undef; // utterly meaningless, but useful for dealing with - // partially optimized code. - } + if (isa(I)) + // utterly meaningless, but useful for dealing with + // partially optimized code. + return I; // Due to inheritance, this must be _after_ the global variable and undef // checks - if (Constant *con = dyn_cast(I)) { + if (Constant *Con = dyn_cast(I)) { assert(!isa(I) && !isa(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"); - return con; + assert(Con->isNullValue() && "null is the only case which makes sense"); + return Con; } if (CastInst *CI = dyn_cast(I)) { - Value *def = CI->stripPointerCasts(); - assert(def->getType()->isPointerTy() && - "Base for pointer must be another pointer"); + Value *Def = CI->stripPointerCasts(); // 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(def) && "shouldn't find another cast here"); - return findBaseDefiningValue(def); + assert(!isa(Def) && "shouldn't find another cast here"); + return findBaseDefiningValue(Def); } - if (LoadInst *LI = dyn_cast(I)) { - if (LI->getType()->isPointerTy()) { - 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(I)) { - Value *Op = GEP->getOperand(0); - if (Op->getType()->isPointerTy()) { - return findBaseDefiningValue(Op); // The base of this GEP is the base - } - } + if (isa(I)) + return I; // The value loaded is an gc base itself - if (AllocaInst *alloc = dyn_cast(I)) { - // An alloca represents a conceptual stack slot. It's the slot itself - // that the GC needs to know about, not the value in the slot. - assert(alloc->getType()->isPointerTy() && - "Base for pointer must be another pointer"); - return alloc; - } + if (GetElementPtrInst *GEP = dyn_cast(I)) + // The base of this GEP is the base + return findBaseDefiningValue(GEP->getPointerOperand()); if (IntrinsicInst *II = dyn_cast(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(I)) { - assert(call->getType()->isPointerTy() && - "Base for pointer must be another pointer"); - return call; - } - if (InvokeInst *invoke = dyn_cast(I)) { - assert(invoke->getType()->isPointerTy() && - "Base for pointer must be another pointer"); - return invoke; - } + if (isa(I) || isa(I)) + return I; // 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(I) && "Landing Pad is unimplemented"); - if (AtomicCmpXchgInst *cas = dyn_cast(I)) { + if (isa(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 - // had better be a valid base pointer. - return cas->getPointerOperand(); - } - if (AtomicRMWInst *atomic = dyn_cast(I)) { - 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(); - } + // like a load. + return I; + + assert(!isa(I) && "Xchg handled above, all others are " + "binary ops which don't apply to pointers"); // 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(I)) { - return ev; - } + if (isa(I)) + 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(I)) { - return select; - } - - return cast(I); + assert((isa(I) || isa(I)) && + "missing instruction case in findBaseDefiningValing"); + return I; } /// Returns the base defining value for this value. -- 2.34.1