[llvm-commits] [PATCH] update getPointerTo to handle multiple address spaces

[Villmow, Micah]

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Eli Friedman
> Sent: Monday, October 29, 2012 1:04 PM
> To: reviews+D85+public+32bd68782163e28d at llvm-reviews.chandlerc.com
> Cc: llvm-commits at cs.uiuc.edu; villmow at gmail.com
> Subject: Re: [llvm-commits] [PATCH] update getPointerTo to handle
> multiple address spaces
> 
> On Fri, Oct 26, 2012 at 3:24 PM, Micah Villmow <villmow at gmail.com>
> wrote:
> > This patch updates getPointerTo to handle multiple address spaces.
> The original request was here:
> > http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-
> 20121022/154243.html
> >
> > http://llvm-reviews.chandlerc.com/D85
> >
> > Files:
> >   lib/Transforms/Vectorize/LoopVectorize.cpp
> >   lib/Transforms/Instrumentation/ThreadSanitizer.cpp
> >   lib/Transforms/Instrumentation/GCOVProfiling.cpp
> >   lib/Transforms/Instrumentation/ProfilingUtils.cpp
> >   lib/Transforms/Scalar/SROA.cpp
> >   lib/VMCore/Constants.cpp
> >   lib/VMCore/Type.cpp
> >   lib/VMCore/Verifier.cpp
> >   lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
> >   lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> >   lib/ExecutionEngine/ExecutionEngine.cpp
> >   include/llvm/Type.h
> >   examples/ExceptionDemo/ExceptionDemo.cpp
> >   tools/clang/lib/CodeGen/ItaniumCXXABI.cpp
> >   tools/clang/lib/CodeGen/CGObjCMac.cpp
> >   tools/clang/lib/CodeGen/CGExprScalar.cpp
> >   tools/clang/lib/CodeGen/CGExpr.cpp
> >   tools/clang/lib/CodeGen/CGVTables.cpp
> >   tools/clang/lib/CodeGen/CGClass.cpp
> >   tools/clang/lib/CodeGen/CGBuiltin.cpp
> >   tools/clang/lib/CodeGen/CGDecl.cpp
> >   tools/clang/lib/CodeGen/CGCXXABI.cpp
> >   tools/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
> >   tools/clang/lib/CodeGen/CGException.cpp
> >   tools/clang/lib/CodeGen/CGCXX.cpp
> >   tools/clang/lib/CodeGen/CGObjC.cpp
> >   tools/clang/lib/CodeGen/CodeGenTypes.cpp
> >   tools/clang/lib/CodeGen/CodeGenModule.cpp
> >   tools/clang/lib/CodeGen/CGExprCXX.cpp
> >   tools/clang/lib/CodeGen/CGCall.cpp
> >   tools/clang/lib/CodeGen/CGBlocks.cpp
> >   tools/clang/lib/CodeGen/CGObjCRuntime.cpp
> >   tools/dragonegg/src/Backend.cpp
> >   tools/dragonegg/src/TypeConversion.cpp
> >   tools/dragonegg/src/ConstantConversion.cpp
> >   tools/dragonegg/src/x86/Target.cpp
> >   tools/dragonegg/src/Convert.cpp
> >   tools/dragonegg/src/DefaultABI.cpp
> >   tools/lldb/source/Expression/IRForTarget.cpp
> 
> You'll want to get Duncan to review the dragonegg bits.
> 
> @@ -2088,7 +2090,8 @@
>      return NSConcreteGlobalBlock;
> 
>    NSConcreteGlobalBlock =
> GetOrCreateLLVMGlobal("_NSConcreteGlobalBlock",
> -                                                Int8PtrTy-
> >getPointerTo(), 0);
> +                                                Int8PtrTy->
> +
> getPointerTo(Int8PtrTy), 0);
>    configureBlocksRuntimeObject(*this, NSConcreteGlobalBlock);
>    return NSConcreteGlobalBlock;
>  }
> @@ -2098,7 +2101,8 @@
>      return NSConcreteStackBlock;
> 
>    NSConcreteStackBlock =
> GetOrCreateLLVMGlobal("_NSConcreteStackBlock",
> -                                               Int8PtrTy-
> >getPointerTo(), 0);
> +                                               Int8PtrTy->
> +
> getPointerTo(Int8PtrTy), 0);
>    configureBlocksRuntimeObject(*this, NSConcreteStackBlock);
>    return NSConcreteStackBlock;
>  }
> 
> Int8PtrTy is by definition in address-space 0.
[Villmow, Micah] Is this a guarantee just in this location? The API getInt8PtrTy can take an address space, which is why I added it.
> 
> @@ -1719,7 +1720,8 @@
>        //   most derived object pointed to by v.
> 
>        // Get the vtable pointer.
> -      llvm::Value *VTable = CGF.GetVTablePtr(Value,
> PtrDiffLTy->getPointerTo());
> +      llvm::Value *VTable = CGF.GetVTablePtr(Value, PtrDiffLTy->
> +
> getPointerTo(PtrDiffLTy));
> 
>        // Get the offset-to-top from the vtable.
>        llvm::Value *OffsetToTop =
> 
> PtrDiffLTy isn't a pointer.
[Villmow, Micah] In this location, it needs a pointer to itself, the only way to do this is by passing itself to the function, unless I add an API getPointerToSelf().
> 
> @@ -1651,8 +1652,8 @@
>  }
> 
>  llvm::Value *CodeGenFunction::EmitCXXTypeidExpr(const CXXTypeidExpr
> *E) {
> -  llvm::Type *StdTypeInfoPtrTy =
> -    ConvertType(E->getType())->getPointerTo();
> +  llvm::Type *StdTypeInfoPtrTy = ConvertType(E->getType());
> +  StdTypeInfoPtrTy = StdTypeInfoPtrTy->getPointerTo(StdTypeInfoPtrTy);
> 
>    if (E->isTypeOperand()) {
>      llvm::Constant *TypeInfo =
> 
> The type of a CXXTypeidExpr isn't a pointer.
[Villmow, Micah] See above, this is the case of getting a pointer to self.
> 
> Index: tools/clang/lib/CodeGen/CGExprCXX.cpp
> ===================================================================
> --- tools/clang/lib/CodeGen/CGExprCXX.cpp
> +++ tools/clang/lib/CodeGen/CGExprCXX.cpp
> @@ -1643,7 +1643,8 @@
>    }
> 
>    llvm::Value *Value = CGF.GetVTablePtr(ThisPtr,
> -                                        StdTypeInfoPtrTy-
> >getPointerTo());
> +                                        StdTypeInfoPtrTy->
> +
> getPointerTo(StdTypeInfoPtrTy));
> 
>    // Load the type info.
>    Value = CGF.Builder.CreateConstInBoundsGEP1_64(Value, -1ULL);
> 
> Again, address-space 0 by definition.
> 
> @@ -1117,7 +1117,7 @@
>        return Entry;
> 
>      // Make sure the result is of the correct type.
> -    return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo());
> +    return llvm::ConstantExpr::getBitCast(Entry, Ty-
> >getPointerTo(Ty));
>    }
> 
>    // This function doesn't have a complete type (for example, the
> return
> 
> Ty isn't a pointer type.
> 
> Again, I would generally be happier if you would hardcode 0 in clang
> instead of picking random types to base the address-space off of; we
> can always fix it later in individual cases.
> 
> Also, the construct Ty->getPointerTo(Ty) is in general fundamentally
> broken: the type of a pointee generally has nothing to do with the
> type of the pointer to it.
[Villmow, Micah] Yeah, I originally had an overload of getPointerTo that would get a pointer to self, but was told that this approach wasn't ideal due to default arguments. So this is where this construct came from. Maybe as I mentioned above, getPointerToSelf works? Basically what I want in the case of Ty->getPointerTy() to return a Ty*.
> 
> -Eli
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits