Support standard DWARF TLS opcode; currently, only PS4 uses it.

[probinson]

The test change builds on the change in D8011, the rest is independent.

[dblaikie]

Do they have the same semantics? (their name "push" versus "form" confuses me a bit)

Have you tried this with other debuggers to see if they support the standard opcode? Perhaps this doesn't need to be PS4-specific?

[probinson]

Do they have the same semantics? (their name "push" versus "form" confuses me a bit)

In PR18423, you report that GCC says they are different.
Although the PR reminds me that the standard op was defined in DWARF 3; if we still support DWARF 2, it should have a version check too.

Have you tried this with other debuggers to see if they support the standard opcode? Perhaps this doesn't need to be PS4-specific?

I have not run any experiments. Making it not PS4-specific would be cool, but the GCC PR makes me hesitant.

[aprantl]

We would like this to become the default on Darwin, too.
Paul, could you please update PR18423 and also include Darwin in the list of platforms?

[aprantl]

To be precise, the check should be

(isDarwin || isPS4) && DwarfVersion >= 3

[echristo]

Let's just check this with gdb and be done with it. I'd prefer not to conditionalize if at all possible.

Thanks.

-eric

[emaste]

I suspect we'll want this on FreeBSD as well when using DWARF>2 (we still default to DWARF2 at present to try to support ancient gdb).

[emaste]

Let's just check this with gdb and be done with it. I'd prefer not to conditionalize if at all possible.

I strongly agree. I've added a link back to this review in the GDB PR https://sourceware.org/bugzilla/show_bug.cgi?id=11616.

[probinson]

In D8018#133761, @emaste wrote:

Let's just check this with gdb and be done with it. I'd prefer not to conditionalize if at all possible.

I strongly agree. I've added a link back to this review in the GDB PR https://sourceware.org/bugzilla/show_bug.cgi?id=11616.

I have GDB 7.7 on my Ubuntu 14.04, it reports "Unhandled dwarf expression opcode 0x9b"
I think we're stuck with the GNU opcode for a while.

It would have to be conditional on DWARF version anyway, for as long as we're willing to support DWARF 2.

[probinson]

New rev that includes Darwin.

[echristo]

Sadly LGTM. One inline comment.

-eric

test/DebugInfo/X86/tls.ll

31 ↗

(On Diff #21204)

Can probably just regexp check for section {{.*}}debug_info and not worry about it.

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

if (GV.getGlobal()->isThreadLocal()) {
         //    within the module's TLS block.
         addExpr(*Loc, dwarf::DW_FORM_udata,
                 Asm->getObjFileLowering().getDebugThreadLocalSymbol(Sym));
       } else {
         addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_GNU_const_index);
         addUInt(*Loc, dwarf::DW_FORM_udata,
                 DD->getAddressPool().getIndex(Sym, /* TLS */ true));
       }
-      // 3) followed by a custom OP to make the debugger do a TLS lookup.
-      addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_GNU_push_tls_address);
+      // 3) followed by an OP to make the debugger do a TLS lookup.
+      addUInt(*Loc, dwarf::DW_FORM_data1,
+              DD->useGNUTLSOpcode() ? dwarf::DW_OP_GNU_push_tls_address
+                                    : dwarf::DW_OP_form_tls_address);
     } else {
       DD->addArangeLabel(SymbolCU(this, Sym));
       addOpAddress(*Loc, Sym);
     }
 
     addBlock(*VariableDIE, dwarf::DW_AT_location, Loc);
     // Add the linkage name.
     StringRef LinkageName = GV.getLinkageName();

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h

class DwarfDebug : public AsmPrinterHandler {
 
   // Whether or not to use AT_ranges for compilation units.
   bool HasCURanges;
 
   // Whether we emitted a function into a section other than the default
   // text.
   bool UsedNonDefaultText;
 
+  // Whether to use the GNU TLS opcode (instead of the standard opcode).
+  bool UseGNUTLSOpcode;
+
   // Version of dwarf we're emitting.
   unsigned DwarfVersion;
 
   // Maps from a type identifier to the actual MDNode.
   DITypeIdentifierMap TypeIdentifierMap;
 
   // DWARF5 Experimental Options
   bool HasDwarfAccelTables;
class DwarfDebug : public AsmPrinterHandler {
   // FIXME: replace this with a map from comp_dir to table so that we can emit
   // multiple tables during LTO each of which uses directory 0, referencing the
   // comp_dir of all the type units that use it.
   MCDwarfDwoLineTable SplitTypeUnitFileTable;
 
   // True iff there are multiple CUs in this module.
   bool SingleCU;
   bool IsDarwin;
+  bool IsPS4;
 
   AddressPool AddrPool;
 
   DwarfAccelTable AccelNames;
   DwarfAccelTable AccelObjC;
   DwarfAccelTable AccelNamespace;
   DwarfAccelTable AccelTypes;
 
public:
   void addArangeLabel(SymbolCU SCU) { ArangeLabels.push_back(SCU); }
 
   /// \brief For symbols that have a size designated (e.g. common symbols),
   /// this tracks that size.
   void setSymbolSize(const MCSymbol *Sym, uint64_t Size) override {
     SymSize[Sym] = Size;
   }
 
+  /// \brief Returns whether to use DW_OP_GNU_push_tls_address, instead of the
+  /// standard DW_OP_form_tls_address opcode
+  bool useGNUTLSOpcode() const { return UseGNUTLSOpcode; }
+
   // Experimental DWARF5 features.
 
   /// \brief Returns whether or not to emit tables that dwarf consumers can
   /// use to accelerate lookup.
   bool useDwarfAccelTables() const { return HasDwarfAccelTables; }
 
   /// \brief Returns whether or not to change the current debug info for the
   /// split dwarf proposal support.

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

static LLVM_CONSTEXPR DwarfAccelTable::Atom TypeAtoms[] = {
     DwarfAccelTable::Atom(dwarf::DW_ATOM_type_flags, dwarf::DW_FORM_data1)};
 
 DwarfDebug::DwarfDebug(AsmPrinter *A, Module *M)
     : Asm(A), MMI(Asm->MMI), PrevLabel(nullptr), GlobalRangeCount(0),
       InfoHolder(A, "info_string", DIEValueAllocator),
       UsedNonDefaultText(false),
       SkeletonHolder(A, "skel_string", DIEValueAllocator),
       IsDarwin(Triple(A->getTargetTriple()).isOSDarwin()),
+      IsPS4(Triple(A->getTargetTriple()).isPS4()),
       AccelNames(DwarfAccelTable::Atom(dwarf::DW_ATOM_die_offset,
                                        dwarf::DW_FORM_data4)),
       AccelObjC(DwarfAccelTable::Atom(dwarf::DW_ATOM_die_offset,
                                       dwarf::DW_FORM_data4)),
       AccelNamespace(DwarfAccelTable::Atom(dwarf::DW_ATOM_die_offset,
                                            dwarf::DW_FORM_data4)),
       AccelTypes(TypeAtoms) {
 
if (DwarfPubSections == Default)
     HasDwarfPubSections = !IsDarwin;
   else
     HasDwarfPubSections = DwarfPubSections == Enable;
 
   unsigned DwarfVersionNumber = Asm->TM.Options.MCOptions.DwarfVersion;
   DwarfVersion = DwarfVersionNumber ? DwarfVersionNumber
                                     : MMI->getModule()->getDwarfVersion();
 
+  // Darwin and PS4 use the standard TLS opcode (defined in DWARF 3).
+  // Everybody else uses GNU's.
+  UseGNUTLSOpcode = !(IsDarwin || IsPS4) || DwarfVersion < 3;
+
   Asm->OutStreamer.getContext().setDwarfVersion(DwarfVersion);
 
   {
     NamedRegionTimer T(DbgTimerName, DWARFGroupName, TimePassesIsEnabled);
     beginModule();
   }
 }
 

llvm/trunk/test/DebugInfo/X86/tls.ll

 ; RUN: llc %s -o - -filetype=asm -O0 -mtriple=x86_64-unknown-linux-gnu \
-; RUN:   | FileCheck --check-prefix=CHECK --check-prefix=SINGLE --check-prefix=SINGLE-64 %s
+; RUN:   | FileCheck --check-prefix=SINGLE --check-prefix=SINGLE-64 --check-prefix=GNUOP %s
 
 ; RUN: llc %s -o - -filetype=asm -O0 -mtriple=i386-linux-gnu \
-; RUN:   | FileCheck --check-prefix=CHECK --check-prefix=SINGLE --check-prefix=SINGLE-32 %s
+; RUN:   | FileCheck --check-prefix=SINGLE --check-prefix=SINGLE-32 --check-prefix=GNUOP %s
 
 ; RUN: llc %s -o - -filetype=asm -O0 -mtriple=x86_64-unknown-linux-gnu -split-dwarf=Enable \
-; RUN:   | FileCheck --check-prefix=CHECK --check-prefix=FISSION %s
+; RUN:   | FileCheck --check-prefix=FISSION --check-prefix=GNUOP %s
 
 ; RUN: llc %s -o - -filetype=asm -O0 -mtriple=x86_64-scei-ps4 \
-; RUN:   | FileCheck --check-prefix=CHECK --check-prefix=SINGLE --check-prefix=SINGLE-64 %s
+; RUN:   | FileCheck --check-prefix=SINGLE --check-prefix=SINGLE-64 --check-prefix=STDOP %s
+
+; RUN: llc %s -o - -filetype=asm -O0 -mtriple=x86_64-apple-darwin \
+; RUN:   | FileCheck --check-prefix=DARWIN --check-prefix=STDOP %s
 
 ; RUN: llc %s -o - -filetype=asm -O0 -mtriple=x86_64-unknown-freebsd \
-; RUN:   | FileCheck --check-prefix=CHECK --check-prefix=SINGLE --check-prefix=SINGLE-64 %s
+; RUN:   | FileCheck --check-prefix=SINGLE --check-prefix=SINGLE-64 --check-prefix=GNUOP %s
 
 ; FIXME: add relocation and DWARF expression support to llvm-dwarfdump & use
 ; that here instead of raw assembly printing
 
 ; FISSION: .section    .debug_info.dwo,
 ; 3 bytes of data in this DW_FORM_block1 representation of the location of 'tls'
 ; FISSION: .byte 3{{ *}}# DW_AT_location
 ; DW_OP_GNU_const_index (0xfx == 252) to refer to the debug_addr table
 ; FISSION-NEXT: .byte 252
 ; an index of zero into the debug_addr table
 ; FISSION-NEXT: .byte 0
 
 ; SINGLE: .section     .debug_info,
+; DARWIN: .section     {{.*}}debug_info,
+
 ; 10 bytes of data in this DW_FORM_block1 representation of the location of 'tls'
 ; SINGLE-64: .byte     10 # DW_AT_location
 ; DW_OP_const8u (0x0e == 14) of address
 ; SINGLE-64-NEXT: .byte        14
 ; SINGLE-64-NEXT: .quad tls@DTPOFF
 
+; DARWIN: .byte     10 ## DW_AT_location
+; DW_OP_const8u (0x0e == 14) of address
+; DARWIN-NEXT: .byte        14
+; DARWIN-NEXT: .quad _tls
+
+; 6 bytes of data in 32-bit mode
 ; SINGLE-32: .byte     6 # DW_AT_location
 ; DW_OP_const4u (0x0e == 12) of address
 ; SINGLE-32-NEXT: .byte        12
 ; SINGLE-32-NEXT: .long tls@DTPOFF
 
 ; DW_OP_GNU_push_tls_address
-; CHECK-NEXT: .byte 224
+; GNUOP-NEXT: .byte 224
+; DW_OP_form_tls_address
+; STDOP-NEXT: .byte 155
 
 ; FISSION: DW_TAG_variable
 ; FISSION: .byte 2 # DW_AT_location
 ; DW_OP_GNU_addr_index
 ; FISSION-NEXT: .byte 251
 ; FISSION-NEXT: .byte 1
 
 ; FISSION: DW_TAG_template_value_parameter