[PATCH] D46147: [NVPTX] Added a feature to use short pointers for const/local/shared AS.

[Benjamin Kramer via Phabricator via llvm-commits]

bkramer added inline comments.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp:855
+                           CodeAddrSpace == NVPTX::PTXLdStInstCode::SHARED);
+  unsigned int PointerSize = (!TM.is64Bit() || UseShortPointers) ? 32 : 64;
+
----------------
There are 4 identical instances of the pointer size computation in this patch, it should really be pulled into its own function.

You can also consider getting the pointer size from the DataLayout instead. Might be cleaner.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp:1255
+  auto PtrVT = UseShortPointer
+                   ? MVT::getIntegerVT(32)
+                   : getPointerTy(DAG.getDataLayout(), GAN->getAddressSpace());
----------------
Why is this needed? The pointer returned by getPointerTy should be right wrt data layout. 


https://reviews.llvm.org/D46147