[PATCH] D29284: [AMDGPU] Lower null pointers in static variable initializer

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/Utils/AMDGPUConstantFoldUtils.cpp:17
+
+bool isNonZeroNullPointer(const Constant *CV) {
+  auto CE = dyn_cast<ConstantExpr>(CV);
----------------
yaxunl wrote:
> arsenm wrote:
> > This doesn't need to be its own file and isn't the right API to be re-used. I think the right place is in AMDGPUTargetMachine because this is logically part of the DataLayout.
> > 
> > The base function should just be the address spaces so it can be used in the DAG and MI
> I took a look of TargetMachine and its derived classes, I did not see a precedence to put such information there. Is it the right place to hold such information?
> 
> Another question. I am not quite sure about the desired API for this function. Do you want something like this?
> 
> 
> ```
> uint64_t getNullPointerValue(unsigned AddrSpace);
> ```
> 
> which returns -1 for private/local address spaces and 0 for other address spaces?
I think so, it's more datalayout/constant related. e.g. getNameWithPrefix.

I think some of this handling is incorrectly in TargetLowering, e.g. D20758.

Something like that seems right


https://reviews.llvm.org/D29284