[PATCH] D11969: Avoid using of DataLayout::getPointerSize() without address space argument in DWARF info generation
Mekhanoshin, Stanislav via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 12 00:04:47 PDT 2015
That's the problem with the test, I know it is generally needed. There are no such targets in the llvm with a data layout like this, and until data layout issue with pointer size is fixed it will hardly appear. HSAIL target where it is needed is under review and not in the tree, but even it does not have proper data layout because of these issues, and then it also does not have debug support in the version under review. It is quite simple to create a test in our private HSAIL BE version, but there is no code in trunk available to run it with. AMDGPU could be the good candidate to write such a test for, but again it cannot afford to switch data layout until these issues are fixed.
Stas
-----Original Message-----
From: Justin Bogner [mailto:justin at justinbogner.com] On Behalf Of Justin Bogner
Sent: Tuesday, August 11, 2015 11:20 PM
To: Stanislav Mekhanoshin via llvm-commits <llvm-commits at lists.llvm.org>
Cc: Mekhanoshin, Stanislav <Stanislav.Mekhanoshin at amd.com>; Arsenault, Matthew <Matthew.Arsenault at amd.com>; mehdi_amini at apple.com; echristo at gmail.com; reviews+D11969+public+c218119e32184906 at reviews.llvm.org
Subject: Re: [PATCH] D11969: Avoid using of DataLayout::getPointerSize() without address space argument in DWARF info generation
Stanislav Mekhanoshin via llvm-commits <llvm-commits at lists.llvm.org>
writes:
> rampitec created this revision.
> rampitec added reviewers: arsenm, joker-eph, echristo.
> rampitec added a subscriber: llvm-commits.
> rampitec set the repository for this revision to rL LLVM.
>
> DataLayout::getPointerSize() supposed to be used with the address
> space argument, though it has default value 0 for the transition
> period. There are the cases when some kind of a general pointer size
> is desirable, such as with debug info. In some places DataLayout is
> used though. This creates problem if target is 64 bit, but some
> address spaces are 32 bit, and notably if address space 0 has 32 bit
> pointers in 64 bit mode. An example of such target is HSAIL.
>
> In this change calls to DataLayout::getPointerSize() are replaced with
> recently added AsmPrinter::getPointerSize() in DWARF debug code. The
> AsmPrinter::getPointerSize() itself is modified to call
> MAI->getPointerSize(). Originally it was calling
> TargetMachine::getPointerSize() which ends up in the
> DataLayout::getPointerSize() with the default argument. Since this
> call is not virtual in both AsmPrinter and TargetMachine that makes a
> little sense to override it in the target implementation for DWARF
> info purposes. The MCAsmInfo implementation has the benefit to query
> member variable PointerSize which can be previously initialized by the
> target, thus target will have control over the returned value.
>
> In LLVM 3.6 many (but not all) of the getPointerSize() references in
> DWARF info were pointing to the MAI implementation, but recently that
> was changed to the new AsmPrinter and TargetMachine functions.
I don't fully understand how DL.getPointerSize() and MAI.getPointerSize() differ, but this change definitely needs a test along with it.
> Repository:
> rL LLVM
>
> http://reviews.llvm.org/D11969
>
> Files:
> lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
> lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>
> Index: lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> +++ lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> @@ -1517,7 +1517,7 @@
> UseOffsets);
>
> Asm->OutStreamer->AddComment("Address Size (in bytes)");
> - Asm->EmitInt8(Asm->getDataLayout().getPointerSize());
> + Asm->EmitInt8(Asm->getPointerSize());
> }
>
> void DwarfUnit::initSection(MCSection *Section) {
> Index: lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> +++ lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> @@ -1589,7 +1589,7 @@
> // Start the dwarf loc section.
> Asm->OutStreamer->SwitchSection(
> Asm->getObjFileLowering().getDwarfLocSection());
> - unsigned char Size = Asm->getDataLayout().getPointerSize();
> + unsigned char Size = Asm->getPointerSize();
> for (const auto &List : DebugLocs.getLists()) {
> Asm->OutStreamer->EmitLabel(List.Label);
> const DwarfCompileUnit *CU = List.CU; @@ -1727,7 +1727,7 @@
> Asm->OutStreamer->SwitchSection(
> Asm->getObjFileLowering().getDwarfARangesSection());
>
> - unsigned PtrSize = Asm->getDataLayout().getPointerSize();
> + unsigned PtrSize = Asm->getPointerSize();
>
> // Build a list of CUs used.
> std::vector<DwarfCompileUnit *> CUs; @@ -1809,7 +1809,7 @@
> Asm->getObjFileLowering().getDwarfRangesSection());
>
> // Size for our labels.
> - unsigned char Size = Asm->getDataLayout().getPointerSize();
> + unsigned char Size = Asm->getPointerSize();
>
> // Grab the specific ranges for the compile units in the module.
> for (const auto &I : CUMap) {
> Index: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> +++ lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> @@ -155,7 +155,7 @@
> // TODO: add debug info for emulated thread local mode.
> } else {
> // FIXME: Make this work with -gsplit-dwarf.
> - unsigned PointerSize = Asm->getDataLayout().getPointerSize();
> + unsigned PointerSize = Asm->getPointerSize();
> assert((PointerSize == 4 || PointerSize == 8) &&
> "Add support for other sizes if necessary");
> // Based on GCC's support for TLS:
> Index: lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
> +++ lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
> @@ -134,7 +134,7 @@
> default:
> llvm_unreachable("Invalid encoded value.");
> case dwarf::DW_EH_PE_absptr:
> - return MF->getDataLayout().getPointerSize();
> + return MAI->getPointerSize();
> case dwarf::DW_EH_PE_udata2:
> return 2;
> case dwarf::DW_EH_PE_udata4:
> Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> @@ -141,7 +141,7 @@
>
> // Do not use the cached DataLayout because some client use it
> without a Module // (llmv-dsymutil, llvm-dwarfdump).
> -unsigned AsmPrinter::getPointerSize() const { return
> TM.getPointerSize(); }
> +unsigned AsmPrinter::getPointerSize() const { return
> +MAI->getPointerSize(); }
>
> const MCSubtargetInfo &AsmPrinter::getSubtargetInfo() const {
> assert(MF && "getSubtargetInfo requires a valid MachineFunction!");
>
>
> Index: lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> +++ lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> @@ -1517,7 +1517,7 @@
> UseOffsets);
>
> Asm->OutStreamer->AddComment("Address Size (in bytes)");
> - Asm->EmitInt8(Asm->getDataLayout().getPointerSize());
> + Asm->EmitInt8(Asm->getPointerSize());
> }
>
> void DwarfUnit::initSection(MCSection *Section) {
> Index: lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> +++ lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> @@ -1589,7 +1589,7 @@
> // Start the dwarf loc section.
> Asm->OutStreamer->SwitchSection(
> Asm->getObjFileLowering().getDwarfLocSection());
> - unsigned char Size = Asm->getDataLayout().getPointerSize();
> + unsigned char Size = Asm->getPointerSize();
> for (const auto &List : DebugLocs.getLists()) {
> Asm->OutStreamer->EmitLabel(List.Label);
> const DwarfCompileUnit *CU = List.CU; @@ -1727,7 +1727,7 @@
> Asm->OutStreamer->SwitchSection(
> Asm->getObjFileLowering().getDwarfARangesSection());
>
> - unsigned PtrSize = Asm->getDataLayout().getPointerSize();
> + unsigned PtrSize = Asm->getPointerSize();
>
> // Build a list of CUs used.
> std::vector<DwarfCompileUnit *> CUs; @@ -1809,7 +1809,7 @@
> Asm->getObjFileLowering().getDwarfRangesSection());
>
> // Size for our labels.
> - unsigned char Size = Asm->getDataLayout().getPointerSize();
> + unsigned char Size = Asm->getPointerSize();
>
> // Grab the specific ranges for the compile units in the module.
> for (const auto &I : CUMap) {
> Index: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> +++ lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> @@ -155,7 +155,7 @@
> // TODO: add debug info for emulated thread local mode.
> } else {
> // FIXME: Make this work with -gsplit-dwarf.
> - unsigned PointerSize = Asm->getDataLayout().getPointerSize();
> + unsigned PointerSize = Asm->getPointerSize();
> assert((PointerSize == 4 || PointerSize == 8) &&
> "Add support for other sizes if necessary");
> // Based on GCC's support for TLS:
> Index: lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
> +++ lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
> @@ -134,7 +134,7 @@
> default:
> llvm_unreachable("Invalid encoded value.");
> case dwarf::DW_EH_PE_absptr:
> - return MF->getDataLayout().getPointerSize();
> + return MAI->getPointerSize();
> case dwarf::DW_EH_PE_udata2:
> return 2;
> case dwarf::DW_EH_PE_udata4:
> Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> @@ -141,7 +141,7 @@
>
> // Do not use the cached DataLayout because some client use it
> without a Module // (llmv-dsymutil, llvm-dwarfdump).
> -unsigned AsmPrinter::getPointerSize() const { return
> TM.getPointerSize(); }
> +unsigned AsmPrinter::getPointerSize() const { return
> +MAI->getPointerSize(); }
>
> const MCSubtargetInfo &AsmPrinter::getSubtargetInfo() const {
> assert(MF && "getSubtargetInfo requires a valid MachineFunction!");
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list