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

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

Thanks Matt! I like this version without the default argument.

I think your guesses about correct address spaces are as good as mine. But I'm not 100% sure about a few diffs where we now use the "program AS" instead of AS=0. See my inline comments.



================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:218
 // (llvm-dsymutil, llvm-dwarfdump).
-unsigned AsmPrinter::getPointerSize() const { return TM.getPointerSize(); }
+unsigned AsmPrinter::getPointerSize() const {
+  return TM.getProgramPointerSize();
----------------
As far as I can see this determines size of dwarf::DW_FORM_ref_addr for DWARF2, as it is used from DIEInteger::SizeOf().
(It is also used in test code in unittests/DebugInfo/DWARF/DwarfGenerator.cpp)

In DWARF2 DW_FORM_ref_addr was (confusingly) defined as being the size of an address on the target machine. Not sure what that means if there are several address spaces with different sizes on the target machine.

In our out-of-tree-target (a DSP) we have `getPointerSize(0) != getProgramPointerSize()`. Although we do not use DWARF2, so I do not think it really matter for us...

You could play it safe and simply do TM.getPointerSize(0), to avoid any functional change in this patch. But using TM.getProgramPointerSize() might actually be more correct. I do not really know.



================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:984
   uint64_t StackSize = FrameInfo.getStackSize();
-  OutStreamer->EmitSymbolValue(FunctionSymbol, TM.getPointerSize());
+  OutStreamer->EmitSymbolValue(FunctionSymbol, TM.getProgramPointerSize());
   OutStreamer->EmitULEB128IntValue(StackSize);
----------------
Seems reasonable to me.


================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:1529
   TypeIndex PointeeTI = getTypeIndex(Ty->getBaseType(), Ty->getClassType());
-  PointerKind PK = Asm->TM.getPointerSize() == 8 ? PointerKind::Near64
-                                                 : PointerKind::Near32;
+  PointerKind PK = getPointerSizeInBytes() == 8 ? PointerKind::Near64
+                                                : PointerKind::Near32;
----------------
Maybe one needs to inspect Ty here to really find out which address space that should be used.
I'd leave it as Asm->TM.getPointerSize(0) if being uncertain (to avoid changing behavior by mistake).


https://reviews.llvm.org/D44162