[llvm-commits] Patch to add address space support to getIntPtrTy
Villmow, Micah
Micah.Villmow at amd.com
Wed Oct 17 09:38:02 PDT 2012
Thanks Eli! Will get an updated patch with yours and chandlers feedback.
Micah
> -----Original Message-----
> From: Eli Friedman [mailto:eli.friedman at gmail.com]
> Sent: Tuesday, October 16, 2012 6:57 PM
> To: Villmow, Micah
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] Patch to add address space support to
> getIntPtrTy
>
> On Tue, Oct 16, 2012 at 9:43 AM, Villmow, Micah <Micah.Villmow at amd.com>
> wrote:
> > Ping!
> >
> >> -----Original Message-----
> >> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> >> bounces at cs.uiuc.edu] On Behalf Of Villmow, Micah
> >> Sent: Monday, October 15, 2012 1:33 PM
> >> To: llvm-commits at cs.uiuc.edu
> >> Subject: [llvm-commits] Patch to add address space support to
> >> getIntPtrTy
> >>
> >> Here is the second patch to update LLVM to support multiple sized
> >> pointers on a per address space basis. This patch handles the
> changes
> >> required for getIntPtrTy itself.
> >>
> >> Thanks,
> >> Micah
>
> Index: lib/Analysis/Lint.cpp
> ===================================================================
> --- lib/Analysis/Lint.cpp (revision 165940)
> +++ lib/Analysis/Lint.cpp (working copy)
> @@ -607,6 +607,8 @@
> // TODO: Look through calls with unique return values.
> // TODO: Look through vector insert/extract/shuffle.
> V = OffsetOk ? GetUnderlyingObject(V, TD) : V->stripPointerCasts();
> + unsigned AS = V->getType()->isPointerTy() ?
> + cast<PointerType>(V->getType())->getAddressSpace() : 0;
>
> This looks wrong.
>
> Index: lib/Analysis/ConstantFolding.cpp
> ===================================================================
> --- lib/Analysis/ConstantFolding.cpp (revision 165941)
> +++ lib/Analysis/ConstantFolding.cpp (working copy)
> @@ -377,10 +377,12 @@
> }
>
> if (ConstantExpr *CE = dyn_cast<ConstantExpr>(C)) {
> - if (CE->getOpcode() == Instruction::IntToPtr &&
> - CE->getOperand(0)->getType() == TD.getIntPtrType(CE-
> >getContext()))
> + if (CE->getOpcode() == Instruction::IntToPtr) {
> + unsigned AS = dyn_cast<PointerType>(CE->getType())-
> >getAddressSpace();
>
> cast<>, not dyn_cast<>
>
> return ReadDataFromGlobal(CE->getOperand(0), ByteOffset, CurPtr,
> BytesLeft, TD);
> + }
> }
>
> // Otherwise, unknown initializer type.
>
> Wrong indentation.
>
> - Type *IntPtrTy = TD->getIntPtrType(ResultTy->getContext());
> + unsigned AS = ResultTy->isPointerTy() ?
> + dyn_cast<PointerType>(ResultTy)->getAddressSpace() : 0;
> + Type *IntPtrTy = TD->getIntPtrType(ResultTy->getContext(), AS);
>
> Again, cast<>, and you can just assert ResultTy->isPointerTy() in the
> context of a GEP.
>
> (I won't continue noting these from here on out.)
>
> @@ -701,6 +706,7 @@
> // This makes it easy to determine if the getelementptr is
> "inbounds".
> // Also, this helps GlobalOpt do SROA on GlobalVariables.
> Type *Ty = Ptr->getType();
> + AS = cast<PointerType>(Ty)->getAddressSpace();
> assert(Ty->isPointerTy() && "Forming regular GEP of non-pointer
> type");
> SmallVector<Constant*, 32> NewIdxs;
> do {
>
> The operand and result of a GEP have to be in the same address-space;
> an assertion at most is appropriate here.
>
> Index: lib/Analysis/ScalarEvolution.cpp
> ===================================================================
> --- lib/Analysis/ScalarEvolution.cpp (revision 165940)
> +++ lib/Analysis/ScalarEvolution.cpp (working copy)
> @@ -2585,8 +2585,10 @@
> // If we have DataLayout, we can bypass creating a target-
> independent
> // constant expression and then folding it back into a ConstantInt.
> // This is just a compile-time optimization.
> + unsigned AS = AllocTy->isPointerTy() ?
> + cast<PointerType>(AllocTy)->getAddressSpace() : 0;
>
> Just use 0 here; even if AllocTy is a pointer, its address-space is
> not the one you want anyway.
>
> Also, getPointerAddressSpace().
>
> @@ -2611,8 +2613,9 @@
> // If we have DataLayout, we can bypass creating a target-
> independent
> // constant expression and then folding it back into a ConstantInt.
> // This is just a compile-time optimization.
> + unsigned AS = 0;
> if (TD)
> - return getConstant(TD->getIntPtrType(getContext()),
> + return getConstant(TD->getIntPtrType(getContext(), AS),
> TD->getStructLayout(STy)-
> >getElementOffset(FieldNo));
>
> Constant *C = ConstantExpr::getOffsetOf(STy, FieldNo);
>
> Useless variable?
>
> @@ -282,8 +282,11 @@
> bool X86FastISel::X86FastEmitStore(EVT VT, const Value *Val,
> const X86AddressMode &AM) {
> // Handle 'null' like i32/i64 0.
> - if (isa<ConstantPointerNull>(Val))
> - Val = Constant::getNullValue(TD.getIntPtrType(Val->getContext()));
> + if (isa<ConstantPointerNull>(Val)) {
> + unsigned AS = Val->getType()->isPointerTy() ?
> + Val->getType()->getPointerAddressSpace() : 0;
> + Val = Constant::getNullValue(TD.getIntPtrType(Val->getContext(),
> AS));
> + }
>
> A ConstantPointerNull is always a pointer.
>
> @@ -894,8 +897,11 @@
> if (Op0Reg == 0) return false;
>
> // Handle 'null' like i32/i64 0.
> - if (isa<ConstantPointerNull>(Op1))
> - Op1 = Constant::getNullValue(TD.getIntPtrType(Op0->getContext()));
> + if (isa<ConstantPointerNull>(Op1)) {
> + unsigned AS = Op1->getType()->isPointerTy() ?
> + Op1->getType()->getPointerAddressSpace() : 0;
> + Op1 = Constant::getNullValue(TD.getIntPtrType(Op0->getContext(),
> AS));
> + }
>
> // We have two options: compare with register or immediate. If the
> RHS of
> // the compare is an immediate that we can fold into this compare,
> use
>
> Same here.
>
> @@ -100,10 +100,13 @@
> return false;
>
> // No-op casts are trivially coalesced by fast-isel.
> - if (const CastInst *Cast = dyn_cast<CastInst>(I))
> - if (Cast->isNoopCast(TD.getIntPtrType(Cast->getContext())) &&
> + if (const CastInst *Cast = dyn_cast<CastInst>(I)) {
> + unsigned AS = V->getType()->isPointerTy() ?
> + V->getType()->getPointerAddressSpace() : 0;
> + if (Cast->isNoopCast(TD.getIntPtrType(Cast->getContext(), AS)) &&
> !hasTrivialKill(Cast->getOperand(0)))
> return false;
> + }
>
> // GEPs with all zero indices are trivially coalesced by fast-isel.
> if (const GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(I))
>
> Wrong address space for some casts.
>
> } else if (isa<ConstantPointerNull>(V)) {
> + unsigned AS = V->getType()->isPointerTy()
> ?V->getType()->getPointerAddressSpace()
> + : 0;
>
> ConstantPointerNull is always a pointer.
>
> Index: lib/Transforms/Utils/SimplifyLibCalls.cpp
> ===================================================================
> --- lib/Transforms/Utils/SimplifyLibCalls.cpp (revision 165940)
> +++ lib/Transforms/Utils/SimplifyLibCalls.cpp (working copy)
> @@ -103,13 +103,15 @@
> this->CI = CI;
> FunctionType *FT = Callee->getFunctionType();
> LLVMContext &Context = CI->getParent()->getContext();
> + unsigned AS0 = cast<PointerType>(FT->getParamType(0))-
> >getAddressSpace();
> + unsigned AS1 = cast<PointerType>(FT->getParamType(1))-
> >getAddressSpace();
>
> // Check if this has the right signature.
> if (FT->getNumParams() != 4 || FT->getReturnType() !=
> FT->getParamType(0) ||
> !FT->getParamType(0)->isPointerTy() ||
> !FT->getParamType(1)->isPointerTy() ||
> - FT->getParamType(2) != TD->getIntPtrType(Context) ||
> - FT->getParamType(3) != TD->getIntPtrType(Context))
> + FT->getParamType(2) != TD->getIntPtrType(Context, AS0) ||
> + FT->getParamType(3) != TD->getIntPtrType(Context, AS1))
> return 0;
>
> You're doing the cast<> too early. Same for other similar changes in
> this file.
>
>
> + unsigned AS = cast<PointerType>(CpyDst->getType())-
> >getAddressSpace();
>
> Again, getPointerAddressSpace.
>
> Index: lib/Transforms/InstCombine/InstCombineCalls.cpp
> ===================================================================
> --- lib/Transforms/InstCombine/InstCombineCalls.cpp (revision 165940)
> +++ lib/Transforms/InstCombine/InstCombineCalls.cpp (working copy)
> @@ -992,13 +992,17 @@
>
> // Check to see if we are changing the return type...
> if (OldRetTy != NewRetTy) {
> + unsigned NewAS = NewRetTy->isPointerTy() ?
> + NewRetTy->getPointerAddressSpace() : 0;
> + unsigned OldAS = OldRetTy->isPointerTy() ?
> + OldRetTy->getPointerAddressSpace() : 0;
> if (Callee->isDeclaration() &&
> // Conversion is ok if changing from one pointer type to
> another or from
> // a pointer to an integer of the same size.
> !((OldRetTy->isPointerTy() || !TD ||
> - OldRetTy == TD->getIntPtrType(Caller->getContext())) &&
> + OldRetTy == TD->getIntPtrType(Caller->getContext(), OldAS))
> &&
> (NewRetTy->isPointerTy() || !TD ||
> - NewRetTy == TD->getIntPtrType(Caller->getContext()))))
> + NewRetTy == TD->getIntPtrType(Caller->getContext(),
> NewAS))))
> return false; // Cannot transform this return value.
>
> if (!Caller->use_empty() &&
>
> Wrong address space.
>
> @@ -1057,11 +1061,15 @@
>
> // Converting from one pointer type to another or between a
> pointer and an
> // integer of the same size is safe even if we do not have a body.
> + unsigned ParamAS = ParamTy->isPointerTy() ?
> + ParamTy->getPointerAddressSpace() : 0;
> + unsigned ActAS = ActTy->isPointerTy() ?
> + ActTy->getPointerAddressSpace() : 0;
> bool isConvertible = ActTy == ParamTy ||
> (TD && ((ParamTy->isPointerTy() ||
> - ParamTy == TD->getIntPtrType(Caller->getContext())) &&
> + ParamTy == TD->getIntPtrType(Caller->getContext(), ParamAS)) &&
> (ActTy->isPointerTy() ||
> - ActTy == TD->getIntPtrType(Caller->getContext()))));
> + ActTy == TD->getIntPtrType(Caller->getContext(),
> ActAS))));
> if (Callee->isDeclaration() && !isConvertible) return false;
> }
>
> Wrong address space.
>
> Index: lib/Transforms/InstCombine/InstructionCombining.cpp
> ===================================================================
> --- lib/Transforms/InstCombine/InstructionCombining.cpp (revision
> 165940)
> +++ lib/Transforms/InstCombine/InstructionCombining.cpp (working
> copy)
> @@ -746,7 +746,8 @@
> // Start with the index over the outer type. Note that the type
> size
> // might be zero (even if the offset isn't zero) if the indexed type
> // is something like [0 x {int, int}]
> - Type *IntPtrTy = TD->getIntPtrType(Ty->getContext());
> + unsigned AS = Ty->isPointerTy() ? Ty->getPointerAddressSpace() : 0;
> + Type *IntPtrTy = TD->getIntPtrType(Ty->getContext(), AS);
> int64_t FirstIdx = 0;
> if (int64_t TySize = TD->getTypeAllocSize(Ty)) {
> FirstIdx = Offset/TySize;
>
> Wrong address space.
>
> Index: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> ===================================================================
> --- lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> (revision 165940)
> +++ lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> (working copy)
> @@ -173,7 +173,9 @@
> // Ensure that the alloca array size argument has type intptr_t, so
> that
> // any casting is exposed early.
> if (TD) {
> - Type *IntPtrTy = TD->getIntPtrType(AI.getContext());
> + unsigned AS = AI.getType()->isPointerTy() ?
> + AI.getType()->getPointerAddressSpace() : 0;
> + Type *IntPtrTy = TD->getIntPtrType(AI.getContext(), AS);
> if (AI.getArraySize()->getType() != IntPtrTy) {
> Value *V = Builder->CreateIntCast(AI.getArraySize(),
> IntPtrTy, false);
>
> The result of alloca is always a pointer, and for the moment always in
> address space 0.
>
> Also, once you've made all those changes, please review it yourself
> one more time.
>
> -Eli
More information about the llvm-commits
mailing list