[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space
Matt Arsenault via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 3 21:56:25 PDT 2016
arsenm added a comment.
In https://reviews.llvm.org/D26196#586433, @rjmccall wrote:
> I think there are a number of conceptual questions here, chiefly among them whether an llvm::ConstantPointerNull in an address-space type should have the correct representation for the target. If the decision for that is "no", then ok, we can produce an explicit representation in IRGen; however, we will need to update quite a bit of code in order to do so correctly.
I think the answer for this is yes, but doing so would be a pretty involved project.
In https://reviews.llvm.org/D26196#586723, @Anastasia wrote:
> My understanding of NULL constant in IR was that it doesn't assume any specific integer value (i.e. 0). And currently as I can see it is lowered very late in LLVM backend during the code selection phase.
>
> Looking at the lowering code in LLVM, it seems like it shouldn't be too difficult to special case the NULL value based on the address space: http://llvm.org/docs/doxygen/html/SelectionDAGBuilder_8cpp_source.html#l01039 instead of using hard-coded 0 at it's done now.
>
> My opinion is that we should try to lower the code as late as possible and Clang doesn't seem like an ideal place for this. Also this change adds extra overhead by adding extra addresspacecast instructions and complicates frontend implementation. I don't see immediate benefit in changing this in the frontend compared to changing LLVM code selection phase instead.
This is essentially moving the lowering of the constant to as late as possible by moving it to the addrspacecast lowering. I think it would be a pretty substantial project to add non-0 null pointer support, but would be better long term. It is treated as isNull, so computeKnownBits and other places already treat null as an alias for 0. Tracking down all of these would be a time consuming endeavor
https://reviews.llvm.org/D26196
More information about the cfe-commits
mailing list