[llvm-commits] Patch to update LLVM/Tools with getPointerTo getting multiple address space support

[Villmow, Micah]

> -----Original Message-----
> From: Eli Friedman [mailto:eli.friedman at gmail.com]
> Sent: Wednesday, October 24, 2012 4:28 PM
> To: Villmow, Micah
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] Patch to update LLVM/Tools with getPointerTo
> getting multiple address space support
> 
> On Wed, Oct 24, 2012 at 2:43 PM, Villmow, Micah <Micah.Villmow at amd.com>
> wrote:
> > Here is a cleaned up version of the patch.
> 
> +  /// getPointerTo - Return a pointer of the current type.
> +  PointerType *getPointerTo();
> 
> I don't see why you're keeping this API around.
[Villmow, Micah] This is a new API that is used quite often to get a pointer to a type based on the type.
For example there is I32PtrTy->getPointerTo()->getPointerTo(). Without this API, I would have to write,
I32PtrTy->getPointerTo(I32PtrTy)->getPointerTo()->(I32PtrTy). The other two API's are only used if you want
to change the address space of the pointer and not use what is already there.
> 
> -    llvm::Type *PtrTy = LLVMCatchTy->getPointerTo(0); // addrspace 0 ok
> +    llvm::Type *PtrTy = LLVMCatchTy->getPointerTo((unsigned)0); //
> addrspace 0 ok
> 
> 0U.
[Villmow, Micah] Ahh yeah!
> 
> Index: tools/clang/lib/CodeGen/CGObjCMac.cpp
> ===================================================================
> --- tools/clang/lib/CodeGen/CGObjCMac.cpp	(revision 166609)
> +++ tools/clang/lib/CodeGen/CGObjCMac.cpp	(working copy)
> @@ -4739,7 +4739,7 @@
>    // ImpnfABITy - LLVM for id (*)(id, SEL, ...)
>    llvm::Type *params[] = { ObjectPtrTy, SelectorPtrTy };
>    ImpnfABITy = llvm::FunctionType::get(ObjectPtrTy, params, false)
> -                 ->getPointerTo();
> +                 ->getPointerTo(ObjectPtrTy);
> 
> The object type has nothing to do with the function type.
> 
> Index: lib/VMCore/Verifier.cpp
> ===================================================================
> --- lib/VMCore/Verifier.cpp	(revision 166609)
> +++ lib/VMCore/Verifier.cpp	(working copy)
> @@ -434,7 +434,8 @@
>      if (ArrayType *ATy = dyn_cast<ArrayType>(GV.getType())) {
>        StructType *STy = dyn_cast<StructType>(ATy->getElementType());
>        PointerType *FuncPtrTy =
> -          FunctionType::get(Type::getVoidTy(*Context), false)-
> >getPointerTo();
> +          FunctionType::get(Type::getVoidTy(*Context), false)
> +          ->getPointerTo(GV.getType());
>        Assert1(STy && STy->getNumElements() == 2 &&
>                STy->getTypeAtIndex(0u)->isIntegerTy(32) &&
>                STy->getTypeAtIndex(1) == FuncPtrTy,
> 
> Same here.
> 
> 
> More generally, in clang, I would prefer to see "0" unless you have a
> reason to believe the address could be a non-default address-space.  A
> lot of the places you're changing are constrained by the C/C++ ABI.
[Villmow, Micah] Ok, I'll go back over these and double check them all.
> 
> -Eli