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:
> > 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.


https://reviews.llvm.org/D32248



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to