[llvm] r234126 - [opaque pointer type] The last of the GEP IRBuilder API migrations

Hal Finkel hfinkel at anl.gov
Mon Apr 6 02:13:49 PDT 2015


----- Original Message -----
> From: "David Blaikie" <dblaikie at gmail.com>
> To: llvm-commits at cs.uiuc.edu
> Sent: Sunday, April 5, 2015 5:41:44 PM
> Subject: [llvm] r234126 - [opaque pointer type] The last of the GEP IRBuilder	API migrations
> 
> Author: dblaikie
> Date: Sun Apr  5 17:41:44 2015
> New Revision: 234126
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=234126&view=rev
> Log:
> [opaque pointer type] The last of the GEP IRBuilder API migrations
> 
> There's still lots of callers passing nullptr, of course - some
> because
> they'll never be migrated (InstCombines for bitcasts - well they
> don't
> make any sense when the pointer type is opaque anyway, for example)
> and
> others that will need more engineering to pass Types around.
> 
> Modified:
>     llvm/trunk/include/llvm/IR/IRBuilder.h
>     llvm/trunk/lib/IR/Core.cpp
>     llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
> 
> Modified: llvm/trunk/include/llvm/IR/IRBuilder.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/IRBuilder.h?rev=234126&r1=234125&r2=234126&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/IRBuilder.h (original)
> +++ llvm/trunk/include/llvm/IR/IRBuilder.h Sun Apr  5 17:41:44 2015
> @@ -1102,16 +1102,20 @@ public:
>    }
>    Value *CreateConstInBoundsGEP2_32(Value *Ptr, unsigned Idx0,
>    unsigned Idx1,
>                                      const Twine &Name = "") {
> +    return CreateConstInBoundsGEP2_32(nullptr, Ptr, Idx0, Idx1,
> Name);
> +  }
> +  Value *CreateConstInBoundsGEP2_32(Type *Ty, Value *Ptr, unsigned
> Idx0, unsigned Idx1,
> +                                    const Twine &Name = "") {

Ty is supposed to be the pointer element type of Ptr, right? How is this API supposed to work if we have a pointer to a pointer to something? If I have i32**, then the pointer element type is i32*, and we get:

  getelementptr i32*, i32** %ptr, i64 0, i64 2

but once this transition is complete, we'll have only:

  getelementptr pointer, pointer %ptr, i64 0, i64 2

and we won't have information on the size of the pointer element's element type, and thus, can't resolve the offset implied by the last index. Are we going to simply forbid using GEPs this way? If so, is that restriction going to be reflected in the IRBuilder interface, or is IRBuilder going to hide that (or make it easier) somehow?

Also, are there any asserts anywhere that trigger if we get these types wrong? I'm thinking of something like

  assert(!Ty || Ty == Ptr->getPointerElementType())

but maybe these are just further down the call chain?

Thanks again,
Hal

>      Value *Idxs[] = {
>        ConstantInt::get(Type::getInt32Ty(Context), Idx0),
>        ConstantInt::get(Type::getInt32Ty(Context), Idx1)
>      };
>  
>      if (Constant *PC = dyn_cast<Constant>(Ptr))
> -      return Insert(Folder.CreateInBoundsGetElementPtr(nullptr, PC,
> Idxs),
> +      return Insert(Folder.CreateInBoundsGetElementPtr(Ty, PC,
> Idxs),
>                      Name);
>  
> -    return Insert(GetElementPtrInst::CreateInBounds(nullptr, Ptr,
> Idxs), Name);
> +    return Insert(GetElementPtrInst::CreateInBounds(Ty, Ptr, Idxs),
> Name);
>    }
>    Value *CreateConstGEP1_64(Value *Ptr, uint64_t Idx0, const Twine
>    &Name = "") {
>      Value *Idx = ConstantInt::get(Type::getInt64Ty(Context), Idx0);
> @@ -1156,7 +1160,10 @@ public:
>      return Insert(GetElementPtrInst::CreateInBounds(nullptr, Ptr,
>      Idxs), Name);
>    }
>    Value *CreateStructGEP(Value *Ptr, unsigned Idx, const Twine &Name
>    = "") {
> -    return CreateConstInBoundsGEP2_32(Ptr, 0, Idx, Name);
> +    return CreateStructGEP(nullptr, Ptr, Idx, Name);
> +  }
> +  Value *CreateStructGEP(Type *Ty, Value *Ptr, unsigned Idx, const
> Twine &Name = "") {
> +    return CreateConstInBoundsGEP2_32(Ty, Ptr, 0, Idx, Name);
>    }
>  
>    /// \brief Same as CreateGlobalString, but return a pointer with
>    "i8*" type
> 
> Modified: llvm/trunk/lib/IR/Core.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Core.cpp?rev=234126&r1=234125&r2=234126&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Core.cpp (original)
> +++ llvm/trunk/lib/IR/Core.cpp Sun Apr  5 17:41:44 2015
> @@ -2518,7 +2518,7 @@ LLVMValueRef LLVMBuildInBoundsGEP(LLVMBu
>  
>  LLVMValueRef LLVMBuildStructGEP(LLVMBuilderRef B, LLVMValueRef
>  Pointer,
>                                  unsigned Idx, const char *Name) {
> -  return wrap(unwrap(B)->CreateStructGEP(unwrap(Pointer), Idx,
> Name));
> +  return wrap(unwrap(B)->CreateStructGEP(nullptr, unwrap(Pointer),
> Idx, Name));
>  }
>  
>  LLVMValueRef LLVMBuildGlobalString(LLVMBuilderRef B, const char
>  *Str,
> 
> Modified:
> llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp?rev=234126&r1=234125&r2=234126&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
> (original)
> +++ llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
> Sun Apr  5 17:41:44 2015
> @@ -1472,17 +1472,16 @@ void DFSanVisitor::visitCallSite(CallSit
>            Args.push_back(DFSF.getShadow(*i));
>  
>          if (FT->isVarArg()) {
> -          auto LabelVAAlloca =
> -              new AllocaInst(ArrayType::get(DFSF.DFS.ShadowTy,
> -                                            CS.arg_size() -
> FT->getNumParams()),
> -                             "labelva",
> DFSF.F->getEntryBlock().begin());
> +          auto *LabelVATy = ArrayType::get(DFSF.DFS.ShadowTy,
> CS.arg_size() - FT->getNumParams());
> +          auto *LabelVAAlloca = new AllocaInst(LabelVATy, "labelva",
> +
>                                               DFSF.F->getEntryBlock().begin());
>  
>            for (unsigned n = 0; i != CS.arg_end(); ++i, ++n) {
> -            auto LabelVAPtr = IRB.CreateStructGEP(LabelVAAlloca, n);
> +            auto LabelVAPtr = IRB.CreateStructGEP(LabelVATy,
> LabelVAAlloca, n);
>              IRB.CreateStore(DFSF.getShadow(*i), LabelVAPtr);
>            }
>  
> -          Args.push_back(IRB.CreateStructGEP(LabelVAAlloca, 0));
> +          Args.push_back(IRB.CreateStructGEP(LabelVATy,
> LabelVAAlloca, 0));
>          }
>  
>          if (!FT->getReturnType()->isVoidTy()) {
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory



More information about the llvm-commits mailing list