[llvm-commits] Patch to add address space support to getIntPtrTy

Eli Friedman eli.friedman at gmail.com
Tue Oct 16 18:57:03 PDT 2012


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