[llvm] r351123 - [opaque pointer types] Update LoadInst creation APIs to consistently
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 17 02:03:55 PST 2019
Hi James,
I notice that the 'const char *NameStr' variants were removed in this patch. That wasn't mentioned in the commit message but I assume this was done because of because of the implicit conversion from const char * to Twine making them seem redundant. However, there's a snag with that as nullptr was valid for the const char * version but it isn't valid in Twine's constructor. So existing code using:
... = new LoadInst(Ptr, nullptr, true);
to get this overload:
LoadInst(Value *Ptr, const char *NameStr = nullptr, bool isVolatile = false, Instruction *InsertBefore = nullptr);
now crashes with a nullptr dereference because of the implicit conversion to Twine and Twine's constructor dereferencing the const char *. Callers can change their nullptr to "" to get the intended behaviour so I'm not sure we need to bring them back, but given that the others were only deprecated it might be worth bringing them back so we can mention this nit in a LLVM_ATTRIBUTUE_DEPRECATED message.
> On 14 Jan 2019, at 13:37, James Y Knight via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: jyknight
> Date: Mon Jan 14 13:37:53 2019
> New Revision: 351123
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351123&view=rev
> Log:
> [opaque pointer types] Update LoadInst creation APIs to consistently
> accept a return-type argument.
>
> Note: this also adds a new C API and soft-deprecates the old C API.
>
> Differential Revision: https://reviews.llvm.org/D56558
>
> Modified:
> llvm/trunk/include/llvm-c/Core.h
> llvm/trunk/include/llvm/IR/IRBuilder.h
> llvm/trunk/include/llvm/IR/Instructions.h
> llvm/trunk/lib/IR/Core.cpp
> llvm/trunk/lib/IR/Instructions.cpp
>
> Modified: llvm/trunk/include/llvm-c/Core.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm-c/Core.h?rev=351123&r1=351122&r2=351123&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm-c/Core.h (original)
> +++ llvm/trunk/include/llvm-c/Core.h Mon Jan 14 13:37:53 2019
> @@ -3597,8 +3597,12 @@ LLVMValueRef LLVMBuildAlloca(LLVMBuilder
> LLVMValueRef LLVMBuildArrayAlloca(LLVMBuilderRef, LLVMTypeRef Ty,
> LLVMValueRef Val, const char *Name);
> LLVMValueRef LLVMBuildFree(LLVMBuilderRef, LLVMValueRef PointerVal);
> +// LLVMBuildLoad is deprecated in favor of LLVMBuildLoad2, in preparation for
> +// opaque pointer types.
> LLVMValueRef LLVMBuildLoad(LLVMBuilderRef, LLVMValueRef PointerVal,
> const char *Name);
> +LLVMValueRef LLVMBuildLoad2(LLVMBuilderRef, LLVMTypeRef Ty,
> + LLVMValueRef PointerVal, const char *Name);
> LLVMValueRef LLVMBuildStore(LLVMBuilderRef, LLVMValueRef Val, LLVMValueRef Ptr);
> LLVMValueRef LLVMBuildGEP(LLVMBuilderRef B, LLVMValueRef Pointer,
> LLVMValueRef *Indices, unsigned NumIndices,
>
> Modified: llvm/trunk/include/llvm/IR/IRBuilder.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/IRBuilder.h?rev=351123&r1=351122&r2=351123&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/IRBuilder.h (original)
> +++ llvm/trunk/include/llvm/IR/IRBuilder.h Mon Jan 14 13:37:53 2019
> @@ -1352,22 +1352,35 @@ public:
> return Insert(new AllocaInst(Ty, DL.getAllocaAddrSpace(), ArraySize), Name);
> }
>
> - /// Provided to resolve 'CreateLoad(Ptr, "...")' correctly, instead of
> + /// Provided to resolve 'CreateLoad(Ty, Ptr, "...")' correctly, instead of
> /// converting the string to 'bool' for the isVolatile parameter.
> - LoadInst *CreateLoad(Value *Ptr, const char *Name) {
> - return Insert(new LoadInst(Ptr), Name);
> - }
> -
> - LoadInst *CreateLoad(Value *Ptr, const Twine &Name = "") {
> - return Insert(new LoadInst(Ptr), Name);
> + LoadInst *CreateLoad(Type *Ty, Value *Ptr, const char *Name) {
> + return Insert(new LoadInst(Ty, Ptr), Name);
> }
>
> LoadInst *CreateLoad(Type *Ty, Value *Ptr, const Twine &Name = "") {
> return Insert(new LoadInst(Ty, Ptr), Name);
> }
>
> + LoadInst *CreateLoad(Type *Ty, Value *Ptr, bool isVolatile,
> + const Twine &Name = "") {
> + return Insert(new LoadInst(Ty, Ptr, Twine(), isVolatile), Name);
> + }
> +
> + // Deprecated [opaque pointer types]
> + LoadInst *CreateLoad(Value *Ptr, const char *Name) {
> + return CreateLoad(Ptr->getType()->getPointerElementType(), Ptr, Name);
> + }
> +
> + // Deprecated [opaque pointer types]
> + LoadInst *CreateLoad(Value *Ptr, const Twine &Name = "") {
> + return CreateLoad(Ptr->getType()->getPointerElementType(), Ptr, Name);
> + }
> +
> + // Deprecated [opaque pointer types]
> LoadInst *CreateLoad(Value *Ptr, bool isVolatile, const Twine &Name = "") {
> - return Insert(new LoadInst(Ptr, nullptr, isVolatile), Name);
> + return CreateLoad(Ptr->getType()->getPointerElementType(), Ptr, isVolatile,
> + Name);
> }
>
> StoreInst *CreateStore(Value *Val, Value *Ptr, bool isVolatile = false) {
> @@ -1377,24 +1390,43 @@ public:
> /// Provided to resolve 'CreateAlignedLoad(Ptr, Align, "...")'
> /// correctly, instead of converting the string to 'bool' for the isVolatile
> /// parameter.
> - LoadInst *CreateAlignedLoad(Value *Ptr, unsigned Align, const char *Name) {
> - LoadInst *LI = CreateLoad(Ptr, Name);
> + LoadInst *CreateAlignedLoad(Type *Ty, Value *Ptr, unsigned Align,
> + const char *Name) {
> + LoadInst *LI = CreateLoad(Ty, Ptr, Name);
> LI->setAlignment(Align);
> return LI;
> }
> - LoadInst *CreateAlignedLoad(Value *Ptr, unsigned Align,
> + LoadInst *CreateAlignedLoad(Type *Ty, Value *Ptr, unsigned Align,
> const Twine &Name = "") {
> - LoadInst *LI = CreateLoad(Ptr, Name);
> + LoadInst *LI = CreateLoad(Ty, Ptr, Name);
> LI->setAlignment(Align);
> return LI;
> }
> - LoadInst *CreateAlignedLoad(Value *Ptr, unsigned Align, bool isVolatile,
> - const Twine &Name = "") {
> - LoadInst *LI = CreateLoad(Ptr, isVolatile, Name);
> + LoadInst *CreateAlignedLoad(Type *Ty, Value *Ptr, unsigned Align,
> + bool isVolatile, const Twine &Name = "") {
> + LoadInst *LI = CreateLoad(Ty, Ptr, isVolatile, Name);
> LI->setAlignment(Align);
> return LI;
> }
>
> + // Deprecated [opaque pointer types]
> + LoadInst *CreateAlignedLoad(Value *Ptr, unsigned Align, const char *Name) {
> + return CreateAlignedLoad(Ptr->getType()->getPointerElementType(), Ptr,
> + Align, Name);
> + }
> + // Deprecated [opaque pointer types]
> + LoadInst *CreateAlignedLoad(Value *Ptr, unsigned Align,
> + const Twine &Name = "") {
> + return CreateAlignedLoad(Ptr->getType()->getPointerElementType(), Ptr,
> + Align, Name);
> + }
> + // Deprecated [opaque pointer types]
> + LoadInst *CreateAlignedLoad(Value *Ptr, unsigned Align, bool isVolatile,
> + const Twine &Name = "") {
> + return CreateAlignedLoad(Ptr->getType()->getPointerElementType(), Ptr,
> + Align, isVolatile, Name);
> + }
> +
> StoreInst *CreateAlignedStore(Value *Val, Value *Ptr, unsigned Align,
> bool isVolatile = false) {
> StoreInst *SI = CreateStore(Val, Ptr, isVolatile);
>
> Modified: llvm/trunk/include/llvm/IR/Instructions.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Instructions.h?rev=351123&r1=351122&r2=351123&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/Instructions.h (original)
> +++ llvm/trunk/include/llvm/IR/Instructions.h Mon Jan 14 13:37:53 2019
> @@ -175,47 +175,58 @@ protected:
> LoadInst *cloneImpl() const;
>
> public:
> - LoadInst(Value *Ptr, const Twine &NameStr, Instruction *InsertBefore);
> - LoadInst(Value *Ptr, const Twine &NameStr, BasicBlock *InsertAtEnd);
> - LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr, bool isVolatile = false,
> + LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr = "",
> Instruction *InsertBefore = nullptr);
> - LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile = false,
> - Instruction *InsertBefore = nullptr)
> - : LoadInst(cast<PointerType>(Ptr->getType())->getElementType(), Ptr,
> - NameStr, isVolatile, InsertBefore) {}
> - LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile,
> + LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr, BasicBlock *InsertAtEnd);
> + LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr, bool isVolatile,
> + Instruction *InsertBefore = nullptr);
> + LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr, bool isVolatile,
> BasicBlock *InsertAtEnd);
> - LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile, unsigned Align,
> - Instruction *InsertBefore = nullptr)
> - : LoadInst(cast<PointerType>(Ptr->getType())->getElementType(), Ptr,
> - NameStr, isVolatile, Align, InsertBefore) {}
> LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr, bool isVolatile,
> unsigned Align, Instruction *InsertBefore = nullptr);
> - LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile,
> + LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr, bool isVolatile,
> unsigned Align, BasicBlock *InsertAtEnd);
> - LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile, unsigned Align,
> - AtomicOrdering Order, SyncScope::ID SSID = SyncScope::System,
> - Instruction *InsertBefore = nullptr)
> - : LoadInst(cast<PointerType>(Ptr->getType())->getElementType(), Ptr,
> - NameStr, isVolatile, Align, Order, SSID, InsertBefore) {}
> LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr, bool isVolatile,
> unsigned Align, AtomicOrdering Order,
> SyncScope::ID SSID = SyncScope::System,
> Instruction *InsertBefore = nullptr);
> - LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile,
> + LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr, bool isVolatile,
> unsigned Align, AtomicOrdering Order, SyncScope::ID SSID,
> BasicBlock *InsertAtEnd);
> - LoadInst(Value *Ptr, const char *NameStr, Instruction *InsertBefore);
> - LoadInst(Value *Ptr, const char *NameStr, BasicBlock *InsertAtEnd);
> - LoadInst(Type *Ty, Value *Ptr, const char *NameStr = nullptr,
> - bool isVolatile = false, Instruction *InsertBefore = nullptr);
> - explicit LoadInst(Value *Ptr, const char *NameStr = nullptr,
> - bool isVolatile = false,
> +
> + // Deprecated [opaque pointer types]
> + explicit LoadInst(Value *Ptr, const Twine &NameStr = "",
> Instruction *InsertBefore = nullptr)
> - : LoadInst(cast<PointerType>(Ptr->getType())->getElementType(), Ptr,
> - NameStr, isVolatile, InsertBefore) {}
> - LoadInst(Value *Ptr, const char *NameStr, bool isVolatile,
> - BasicBlock *InsertAtEnd);
> + : LoadInst(Ptr->getType()->getPointerElementType(), Ptr, NameStr,
> + InsertBefore) {}
> + LoadInst(Value *Ptr, const Twine &NameStr, BasicBlock *InsertAtEnd)
> + : LoadInst(Ptr->getType()->getPointerElementType(), Ptr, NameStr,
> + InsertAtEnd) {}
> + LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile,
> + Instruction *InsertBefore = nullptr)
> + : LoadInst(Ptr->getType()->getPointerElementType(), Ptr, NameStr,
> + isVolatile, InsertBefore) {}
> + LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile,
> + BasicBlock *InsertAtEnd)
> + : LoadInst(Ptr->getType()->getPointerElementType(), Ptr, NameStr,
> + isVolatile, InsertAtEnd) {}
> + LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile, unsigned Align,
> + Instruction *InsertBefore = nullptr)
> + : LoadInst(Ptr->getType()->getPointerElementType(), Ptr, NameStr,
> + isVolatile, Align, InsertBefore) {}
> + LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile, unsigned Align,
> + BasicBlock *InsertAtEnd)
> + : LoadInst(Ptr->getType()->getPointerElementType(), Ptr, NameStr,
> + isVolatile, Align, InsertAtEnd) {}
> + LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile, unsigned Align,
> + AtomicOrdering Order, SyncScope::ID SSID = SyncScope::System,
> + Instruction *InsertBefore = nullptr)
> + : LoadInst(Ptr->getType()->getPointerElementType(), Ptr, NameStr,
> + isVolatile, Align, Order, SSID, InsertBefore) {}
> + LoadInst(Value *Ptr, const Twine &NameStr, bool isVolatile, unsigned Align,
> + AtomicOrdering Order, SyncScope::ID SSID, BasicBlock *InsertAtEnd)
> + : LoadInst(Ptr->getType()->getPointerElementType(), Ptr, NameStr,
> + isVolatile, Align, Order, SSID, InsertAtEnd) {}
>
> /// Return true if this is a load from a volatile memory location.
> bool isVolatile() const { return getSubclassDataFromInstruction() & 1; }
>
> Modified: llvm/trunk/lib/IR/Core.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Core.cpp?rev=351123&r1=351122&r2=351123&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Core.cpp (original)
> +++ llvm/trunk/lib/IR/Core.cpp Mon Jan 14 13:37:53 2019
> @@ -3346,7 +3346,15 @@ LLVMValueRef LLVMBuildFree(LLVMBuilderRe
>
> LLVMValueRef LLVMBuildLoad(LLVMBuilderRef B, LLVMValueRef PointerVal,
> const char *Name) {
> - return wrap(unwrap(B)->CreateLoad(unwrap(PointerVal), Name));
> + Value *V = unwrap(PointerVal);
> + PointerType *Ty = cast<PointerType>(V->getType());
> +
> + return wrap(unwrap(B)->CreateLoad(Ty->getElementType(), V, Name));
> +}
> +
> +LLVMValueRef LLVMBuildLoad2(LLVMBuilderRef B, LLVMTypeRef Ty,
> + LLVMValueRef PointerVal, const char *Name) {
> + return wrap(unwrap(B)->CreateLoad(unwrap(Ty), unwrap(PointerVal), Name));
> }
>
> LLVMValueRef LLVMBuildStore(LLVMBuilderRef B, LLVMValueRef Val,
>
> Modified: llvm/trunk/lib/IR/Instructions.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Instructions.cpp?rev=351123&r1=351122&r2=351123&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Instructions.cpp (original)
> +++ llvm/trunk/lib/IR/Instructions.cpp Mon Jan 14 13:37:53 2019
> @@ -1138,28 +1138,30 @@ void LoadInst::AssertOK() {
> "Alignment required for atomic load");
> }
>
> -LoadInst::LoadInst(Value *Ptr, const Twine &Name, Instruction *InsertBef)
> - : LoadInst(Ptr, Name, /*isVolatile=*/false, InsertBef) {}
> +LoadInst::LoadInst(Type *Ty, Value *Ptr, const Twine &Name,
> + Instruction *InsertBef)
> + : LoadInst(Ty, Ptr, Name, /*isVolatile=*/false, InsertBef) {}
>
> -LoadInst::LoadInst(Value *Ptr, const Twine &Name, BasicBlock *InsertAE)
> - : LoadInst(Ptr, Name, /*isVolatile=*/false, InsertAE) {}
> +LoadInst::LoadInst(Type *Ty, Value *Ptr, const Twine &Name,
> + BasicBlock *InsertAE)
> + : LoadInst(Ty, Ptr, Name, /*isVolatile=*/false, InsertAE) {}
>
> LoadInst::LoadInst(Type *Ty, Value *Ptr, const Twine &Name, bool isVolatile,
> Instruction *InsertBef)
> : LoadInst(Ty, Ptr, Name, isVolatile, /*Align=*/0, InsertBef) {}
>
> -LoadInst::LoadInst(Value *Ptr, const Twine &Name, bool isVolatile,
> +LoadInst::LoadInst(Type *Ty, Value *Ptr, const Twine &Name, bool isVolatile,
> BasicBlock *InsertAE)
> - : LoadInst(Ptr, Name, isVolatile, /*Align=*/0, InsertAE) {}
> + : LoadInst(Ty, Ptr, Name, isVolatile, /*Align=*/0, InsertAE) {}
>
> LoadInst::LoadInst(Type *Ty, Value *Ptr, const Twine &Name, bool isVolatile,
> unsigned Align, Instruction *InsertBef)
> : LoadInst(Ty, Ptr, Name, isVolatile, Align, AtomicOrdering::NotAtomic,
> SyncScope::System, InsertBef) {}
>
> -LoadInst::LoadInst(Value *Ptr, const Twine &Name, bool isVolatile,
> +LoadInst::LoadInst(Type *Ty, Value *Ptr, const Twine &Name, bool isVolatile,
> unsigned Align, BasicBlock *InsertAE)
> - : LoadInst(Ptr, Name, isVolatile, Align, AtomicOrdering::NotAtomic,
> + : LoadInst(Ty, Ptr, Name, isVolatile, Align, AtomicOrdering::NotAtomic,
> SyncScope::System, InsertAE) {}
>
> LoadInst::LoadInst(Type *Ty, Value *Ptr, const Twine &Name, bool isVolatile,
> @@ -1174,12 +1176,11 @@ LoadInst::LoadInst(Type *Ty, Value *Ptr,
> setName(Name);
> }
>
> -LoadInst::LoadInst(Value *Ptr, const Twine &Name, bool isVolatile,
> - unsigned Align, AtomicOrdering Order,
> - SyncScope::ID SSID,
> +LoadInst::LoadInst(Type *Ty, Value *Ptr, const Twine &Name, bool isVolatile,
> + unsigned Align, AtomicOrdering Order, SyncScope::ID SSID,
> BasicBlock *InsertAE)
> - : UnaryInstruction(cast<PointerType>(Ptr->getType())->getElementType(),
> - Load, Ptr, InsertAE) {
> + : UnaryInstruction(Ty, Load, Ptr, InsertAE) {
> + assert(Ty == cast<PointerType>(Ptr->getType())->getElementType());
> setVolatile(isVolatile);
> setAlignment(Align);
> setAtomic(Order, SSID);
> @@ -1187,48 +1188,6 @@ LoadInst::LoadInst(Value *Ptr, const Twi
> setName(Name);
> }
>
> -LoadInst::LoadInst(Value *Ptr, const char *Name, Instruction *InsertBef)
> - : UnaryInstruction(cast<PointerType>(Ptr->getType())->getElementType(),
> - Load, Ptr, InsertBef) {
> - setVolatile(false);
> - setAlignment(0);
> - setAtomic(AtomicOrdering::NotAtomic);
> - AssertOK();
> - if (Name && Name[0]) setName(Name);
> -}
> -
> -LoadInst::LoadInst(Value *Ptr, const char *Name, BasicBlock *InsertAE)
> - : UnaryInstruction(cast<PointerType>(Ptr->getType())->getElementType(),
> - Load, Ptr, InsertAE) {
> - setVolatile(false);
> - setAlignment(0);
> - setAtomic(AtomicOrdering::NotAtomic);
> - AssertOK();
> - if (Name && Name[0]) setName(Name);
> -}
> -
> -LoadInst::LoadInst(Type *Ty, Value *Ptr, const char *Name, bool isVolatile,
> - Instruction *InsertBef)
> - : UnaryInstruction(Ty, Load, Ptr, InsertBef) {
> - assert(Ty == cast<PointerType>(Ptr->getType())->getElementType());
> - setVolatile(isVolatile);
> - setAlignment(0);
> - setAtomic(AtomicOrdering::NotAtomic);
> - AssertOK();
> - if (Name && Name[0]) setName(Name);
> -}
> -
> -LoadInst::LoadInst(Value *Ptr, const char *Name, bool isVolatile,
> - BasicBlock *InsertAE)
> - : UnaryInstruction(cast<PointerType>(Ptr->getType())->getElementType(),
> - Load, Ptr, InsertAE) {
> - setVolatile(isVolatile);
> - setAlignment(0);
> - setAtomic(AtomicOrdering::NotAtomic);
> - AssertOK();
> - if (Name && Name[0]) setName(Name);
> -}
> -
> void LoadInst::setAlignment(unsigned Align) {
> assert((Align & (Align-1)) == 0 && "Alignment is not a power of 2!");
> assert(Align <= MaximumAlignment &&
> @@ -3879,7 +3838,7 @@ AllocaInst *AllocaInst::cloneImpl() cons
> }
>
> LoadInst *LoadInst::cloneImpl() const {
> - return new LoadInst(getOperand(0), Twine(), isVolatile(),
> + return new LoadInst(getType(), getOperand(0), Twine(), isVolatile(),
> getAlignment(), getOrdering(), getSyncScopeID());
> }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list