[llvm] r292624 - [mips] Fix debug information for __thread variable

Petar Jovanovic via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 8 09:34:10 PST 2017


AFAICS, DIEExpr seems to be used for TLS values only today. So, if someone decides to use it for different purposes, then this code may need to be specialized.
As of the naming issue, the function that does the same thing in GCC is called 'output_dwarf_dtprel'.

________________________________
From: David Blaikie [dblaikie at gmail.com]
Sent: Wednesday, February 08, 2017 5:36 PM
To: Simon Dardis; Petar Jovanovic; llvm-commits at lists.llvm.org; Eric Christopher; Adrian Prantl; Aleksandar Beserminji
Subject: Re: [llvm] r292624 - [mips] Fix debug information for __thread variable

Is it used/usable for all values? I'm not entirely sure. The call looks pretty unconditional in a fairly generic place.

The implementation looks to me like it only produces TLs values in MipsAsmPrinter::EmitDebugValue - maybe I'm misreading it? (the "DTP" bit in the calls inside the implementation sound thread-ish to me (also going off what the test case tests for, etc) even though I don't actually know what that acronym stands for)

On Wed, Feb 8, 2017 at 8:25 AM Simon Dardis <Simon.Dardis at imgtec.com<mailto:Simon.Dardis at imgtec.com>> wrote:
+CCing original author.

When I reviewed the patch, it appeared to me that it was far too target specific to call the
change to the ASMPrinter EmitDebugThreadLocal because as far as I could see, no other
target required such directives.

Perhaps changing it to EmitTargetDebugValue along with updating the comment to reflect
that some targets have additional requirements for debug information?

Thanks,
Simon
________________________________
From: David Blaikie [dblaikie at gmail.com<mailto:dblaikie at gmail.com>]
Sent: 08 February 2017 16:12
To: Petar Jovanovic; llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>; Eric Christopher; Adrian Prantl; Simon Dardis

Subject: Re: [llvm] r292624 - [mips] Fix debug information for __thread variable
Simon: What's the motivation for that name, given the specific role the function appears to have (at least according to the doc comment)?

Others (Adrian, mostly): Ideas/preferences?

On Wed, Feb 8, 2017 at 7:05 AM Petar Jovanovic <Petar.Jovanovic at imgtec.com<mailto:Petar.Jovanovic at imgtec.com>> wrote:
> Seems the 'EmitDebugValue' should probably have some mention of 'Thread' in it, given the comment describes this as specifically for thread locals?

In the original change (see [1]), this was named EmitDebugThreadLocal, but one of the reviewers requested this to be renamed to EmitDebugValue.

[1] https://reviews.llvm.org/D28770?id=84545#inline-249536

________________________________
From: David Blaikie [dblaikie at gmail.com<mailto:dblaikie at gmail.com>]
Sent: Monday, February 06, 2017 9:54 PM
To: Petar Jovanovic; llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>; Eric Christopher; Adrian Prantl
Subject: Re: [llvm] r292624 - [mips] Fix debug information for __thread variable

Seems the 'EmitDebugValue' should probably have some mention of 'Thread' in it, given the comment describes this as specifically for thread locals?

On Fri, Jan 20, 2017 at 10:04 AM Petar Jovanovic via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
Author: petarj
Date: Fri Jan 20 11:53:30 2017
New Revision: 292624

URL: http://llvm.org/viewvc/llvm-project?rev=292624&view=rev
Log:
[mips] Fix debug information for __thread variable

This patch fixes debug information for __thread variable on Mips
using .dtprelword and .dtpreldword directives.

Patch by Aleksandar Beserminji.

Differential Revision: http://reviews.llvm.org/D28770

Added:
    llvm/trunk/test/DebugInfo/Mips/tls.ll
Modified:
    llvm/trunk/include/llvm/CodeGen/AsmPrinter.h
    llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
    llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
    llvm/trunk/lib/Target/Mips/MipsAsmPrinter.cpp
    llvm/trunk/lib/Target/Mips/MipsAsmPrinter.h
    llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.cpp
    llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.h

Modified: llvm/trunk/include/llvm/CodeGen/AsmPrinter.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/AsmPrinter.h?rev=292624&r1=292623&r2=292624&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/AsmPrinter.h (original)
+++ llvm/trunk/include/llvm/CodeGen/AsmPrinter.h Fri Jan 20 11:53:30 2017
@@ -480,6 +480,12 @@ public:
   /// Get the value for DW_AT_APPLE_isa. Zero if no isa encoding specified.
   virtual unsigned getISAEncoding() { return 0; }

+  /// Emit the directive and value for debug thread local expression
+  ///
+  /// \p Value - The value to emit.
+  /// \p Size - The size of the integer (in bytes) to emit.
+  virtual void EmitDebugValue(const MCExpr *Value, unsigned Size) const;
+
   //===------------------------------------------------------------------===//
   // Dwarf Lowering Routines
   //===------------------------------------------------------------------===//

Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=292624&r1=292623&r2=292624&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (original)
+++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp Fri Jan 20 11:53:30 2017
@@ -567,6 +567,15 @@ void AsmPrinter::EmitGlobalVariable(cons
   OutStreamer->AddBlankLine();
 }

+/// Emit the directive and value for debug thread local expression
+///
+/// \p Value - The value to emit.
+/// \p Size - The size of the integer (in bytes) to emit.
+void AsmPrinter::EmitDebugValue(const MCExpr *Value,
+                                      unsigned Size) const {
+  OutStreamer->EmitValue(Value, Size);
+}
+
 /// EmitFunctionHeader - This method emits the header for the current
 /// function.
 void AsmPrinter::EmitFunctionHeader() {

Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp?rev=292624&r1=292623&r2=292624&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp (original)
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp Fri Jan 20 11:53:30 2017
@@ -484,7 +484,7 @@ void DIEInteger::print(raw_ostream &O) c
 /// EmitValue - Emit expression value.
 ///
 void DIEExpr::EmitValue(const AsmPrinter *AP, dwarf::Form Form) const {
-  AP->OutStreamer->EmitValue(Expr, SizeOf(AP, Form));
+  AP->EmitDebugValue(Expr, SizeOf(AP, Form));
 }

 /// SizeOf - Determine size of expression value in bytes.

Modified: llvm/trunk/lib/Target/Mips/MipsAsmPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsAsmPrinter.cpp?rev=292624&r1=292623&r2=292624&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/MipsAsmPrinter.cpp (original)
+++ llvm/trunk/lib/Target/Mips/MipsAsmPrinter.cpp Fri Jan 20 11:53:30 2017
@@ -1037,6 +1037,22 @@ void MipsAsmPrinter::PrintDebugValueComm
   // TODO: implement
 }

+// Emit .dtprelword or .dtpreldword directive
+// and value for debug thread local expression.
+void MipsAsmPrinter::EmitDebugValue(const MCExpr *Value,
+                                          unsigned Size) const {
+  switch (Size) {
+  case 4:
+    OutStreamer->EmitDTPRel32Value(Value);
+    break;
+  case 8:
+    OutStreamer->EmitDTPRel64Value(Value);
+    break;
+  default:
+    llvm_unreachable("Unexpected size of expression value.");
+  }
+}
+
 // Align all targets of indirect branches on bundle size.  Used only if target
 // is NaCl.
 void MipsAsmPrinter::NaClAlignIndirectJumpTargets(MachineFunction &MF) {

Modified: llvm/trunk/lib/Target/Mips/MipsAsmPrinter.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsAsmPrinter.h?rev=292624&r1=292623&r2=292624&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/MipsAsmPrinter.h (original)
+++ llvm/trunk/lib/Target/Mips/MipsAsmPrinter.h Fri Jan 20 11:53:30 2017
@@ -140,6 +140,7 @@ public:
   void EmitStartOfAsmFile(Module &M) override;
   void EmitEndOfAsmFile(Module &M) override;
   void PrintDebugValueComment(const MachineInstr *MI, raw_ostream &OS);
+  void EmitDebugValue(const MCExpr *Value, unsigned Size) const override;
 };
 }


Modified: llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.cpp?rev=292624&r1=292623&r2=292624&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.cpp (original)
+++ llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.cpp Fri Jan 20 11:53:30 2017
@@ -148,3 +148,11 @@ MCSection *MipsTargetObjectFile::getSect
   // Otherwise, we work the same as ELF.
   return TargetLoweringObjectFileELF::getSectionForConstant(DL, Kind, C, Align);
 }
+
+const MCExpr *
+MipsTargetObjectFile::getDebugThreadLocalSymbol(const MCSymbol *Sym) const {
+  const MCExpr *Expr =
+      MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_None, getContext());
+  return MCBinaryExpr::createAdd(
+      Expr, MCConstantExpr::create(0x8000, getContext()), getContext());
+}

Modified: llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.h?rev=292624&r1=292623&r2=292624&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.h (original)
+++ llvm/trunk/lib/Target/Mips/MipsTargetObjectFile.h Fri Jan 20 11:53:30 2017
@@ -42,6 +42,8 @@ class MipsTargetMachine;
     MCSection *getSectionForConstant(const DataLayout &DL, SectionKind Kind,
                                      const Constant *C,
                                      unsigned &Align) const override;
+    /// Describe a TLS variable address within debug info.
+    const MCExpr *getDebugThreadLocalSymbol(const MCSymbol *Sym) const override;
   };
 } // end namespace llvm


Added: llvm/trunk/test/DebugInfo/Mips/tls.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/Mips/tls.ll?rev=292624&view=auto
==============================================================================
--- llvm/trunk/test/DebugInfo/Mips/tls.ll (added)
+++ llvm/trunk/test/DebugInfo/Mips/tls.ll Fri Jan 20 11:53:30 2017
@@ -0,0 +1,22 @@
+; RUN: llc -O0 -march=mips -mcpu=mips32r2 -filetype=asm < %s | FileCheck %s -check-prefix=CHECK-WORD
+; RUN: llc -O0 -march=mips64 -mcpu=mips64r2 -filetype=asm < %s | FileCheck %s -check-prefix=CHECK-DWORD
+
+ at x = thread_local global i32 5, align 4, !dbg !0
+
+; CHECK-WORD: .dtprelword x+32768
+; CHECK-DWORD: .dtpreldword x+32768
+
+!llvm.dbg.cu<http://llvm.dbg.cu> = !{!2}
+!llvm.module.flags = !{!7, !8}
+!llvm.ident = !{!9}
+
+!0 = !DIGlobalVariableExpression(var: !1)
+!1 = distinct !DIGlobalVariable(name: "x", scope: !2, file: !3, line: 1, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 4.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
+!3 = !DIFile(filename: "tls.c", directory: "/tmp")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!7 = !{i32 2, !"Dwarf Version", i32 4}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+!9 = !{!"clang version 4.0.0"}


_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170208/0357ab1e/attachment.html>


More information about the llvm-commits mailing list