rjmccall added inline comments.
================
Comment at: lib/CodeGen/TargetInfo.cpp:7296
+ unsigned getASTAllocaAddressSpace() const override {
+ return LangAS::Count + getABIInfo().getDataLayout().getAllocaAddrSpace();
+ }
----------------
yaxunl wrote:
> rjmccall wrote:
> > yaxunl wrote:
> > > rjmccall wrote:
> > > > Can we rename LangAS::Count to something more meaningful like
> > > > LangAS::FirstTargetAddressSpace? I think this would clarify a lot of
> > > > code.
> > > >
> > > > Is the OpenCL special case in ASTContext::getTargetAddressSpace still
> > > > correct with this patch? A pointer in LangAS::Default should be
> > > > lowered as a generic pointer instead of a pointer into the alloca
> > > > address space, right?
> > > Will do.
> > >
> > > The OpenCL special case in ASTContext::getTargetAddressSpace is still
> > > correct with this patch. In OpenCL private address space is still
> > > represented in AST by LangAS::Default, so a pointer in LangAS::Default
> > > should be lowered as a pointer to alloca address space.
> > Then I don't understand. If __private is always the alloca address space,
> > why does IRGen have to add addrspace casts after allocating local variables
> > in it? IRGen's invariant is that the value stored in LocalDeclMap for a
> > variable of AST type T is a value of type [[T]] addrspace([[AS]]) *, where
> > [[T]] is the lowered IR type for T and [[AS]] is the target address space
> > of the variable. For auto variables, AS is always LangAS::Default, which
> > according to you means that [[AS]] is always LLVM's notion of the alloca
> > address space, which is exactly the type produced by alloca. So why is
> > there ever a cast?
> >
> > My understanding was that, on your target, __private is (at least
> > sometimes) a 64-bit extension of the alloca address space, which is 32-bit.
> > But that makes __private a different address space from the alloca address
> > space, so the special case in getTargetAddressSpace is still wrong, because
> > that special case means that a pointer-to-__private type will be lowered to
> > be a 32-bit pointer type.
> >
> > Also, I'm not sure why the normal AddrSpaceMap mechanism isn't sufficient
> > for mapping LangAS::Default.
> This work is mostly for C++.
>
> For OpenCL, the default address space is mapped to alloca address space (5),
> there is no address space cast inserted.
>
> For C++, the default address space is mapped to generic address space (0),
> therefore the address space cast is needed.
Wait, I think I might understand. You're saying that, when you're compiling
OpenCL, you want __private to just be the normal 32-bit address space, but when
you're compiling other languages, you want LangAS::Default to be this 64-bit
extension, probably because those other languages don't break things down in
address spaces as consistently as OpenCL and so it's more important for
LangAS::Default to be able to reach across address spaces.
I do not feel that it's correct for this to be hard-coded in ASTContext; it
really seems like a target-specific policy that should just be reflected in
AddrSpaceMap. That does mean that you'll have to find some way of informing
your TargetInfo that it's building OpenCL. The normal design for something
like that would be a target option that the driver would automatically set when
it recognized that it was building OpenCL; or you could change
getAddressSpaceMap to take a LangOptions.
https://reviews.llvm.org/D32248
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
