[PATCH] D44162: TargetMachine: Add address space to getPointerSize

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: include/llvm/Target/TargetMachine.h:141
   /// This is the only time the DataLayout in the TargetMachine is used.
-  unsigned getPointerSize() const { return DL.getPointerSize(); }
+  unsigned getPointerSize(unsigned AS = 0) const {
+    return DL.getPointerSize(AS);
----------------
arsenm wrote:
> bjope wrote:
> > Can we skip the default =0 for these?
> > 
> > If I remember correclty there are FIXME comments in DataLayout about removing the defaults in DataLayout. I think the idea is that people often forget to provide the correct address space. If an address space is mandatory one needs to motivate at a specific call site if it for example is correct to always use 0 instead of looking up the address space for the type. The amount of call sites where it always is correct to use a hard-coded 0 is probably very few.
> I would like to remove the default, but the context where this was used I don't see where the pointer is coming from (i.e. a few places AsmPrinter).
> 
> I'm doing this because it seems like the TargetMachine is the natural place to check the pointer size in GlobalISel's LegalizeInfo
My idea was to, for example, replace all old calls to this method into:
 `getPointerSize(0 /* TODO: Check/describe why address space 0 is correct here*/)`
and, by that removing the default setting, and pushing the TODO down to the call sites.


https://reviews.llvm.org/D44162