[PATCH] Minor cleanup for GNU TLS support

David Blaikie dblaikie at gmail.com
Mon Oct 7 10:26:13 PDT 2013


On Mon, Oct 7, 2013 at 10:12 AM, Eric Christopher <echristo at gmail.com>wrote:

> On Mon, Oct 7, 2013 at 10:00 AM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > On Mon, Oct 7, 2013 at 9:53 AM, Eric Christopher <echristo at gmail.com>
> wrote:
> >>
> >> On Sat, Oct 5, 2013 at 6:07 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >> >
> >> >
> >> >
> >> > On Sat, Oct 5, 2013 at 5:45 PM, Richard Mitton <
> richard at codersnotes.com>
> >> > wrote:
> >> >>
> >> >> It's better without the *user in the switch, as the code that calls
> it
> >> >> will then just print 'invalid opcode %x' or whatever, as opposed to
> >> >> right
> >> >> now, where it knows about DW_OP_lo_user+0, but not any other user OPs
> >> >> (DW_OP_lo_user+N), which is silly.
> >> >>
> >> >> I don't think there's anything that really uses this much already,
> but
> >> >> I'll see if I can figure something out to test it with and the update
> >> >> the
> >> >> patch
> >> >
> >> >
> >> > When we print assembly (rather than the usual debug info of emitting
> an
> >> > object file and then using llvm-dwarfdump) we annotate the assembly
> with
> >> > comments - but thinking about it, I don't think we annotate DWARF
> >> > expressions/operators/etc, so this probably isn't visible. I wonder if
> >> > we
> >> > call the OperationEncodingString at all.
> >> >
> >>
> >> We may not, but it'll be nice to do so. It's handy to use when debugging
> >> anyhow.
> >
> >
> > Guessing: I suspect we can't really do it easily at the moment because
> the
> > dumping code would have to get a lot smarter. The form doesn't imply
> whether
> > or not the bytes are a DWARF expression or not, does it? (ie: data1 may
> or
> > may not be a DWARF expression, it might instead be a file/line number,
> > etc... ) so we might end up annotating things that aren't DWARF
> expressions
> > as though they are. I guess maybe we could have a table of attribute
> kind +
> > form to decide whether to annotate the bytes as DWARF expressions or not.
> >
>
> Oh sure, I actually meant when debugging though. "Please print this
> random number as a DW_OP string" :)
>

Ah, sure. Just untestable (short of a rather trivial unit test) - so I'll
go ahead and commit this patch as-is.


>
> -eric
>
> >>
> >>
> >> -eric
> >>
> >> >>
> >> >> .
> >> >>
> >> >>
> >> >> Eric Christopher wrote:
> >> >>>
> >> >>> Should dump differently I'd have thought. Also don't want to delete
> >> >>> the DW_OP_*_user text I'd think.
> >> >>>
> >> >>> -eric
> >> >>>
> >> >>> On Sat, Oct 5, 2013 at 2:33 PM, Richard
> >> >>> Mitton<richard at codersnotes.com>
> >> >>> wrote:
> >> >>>
> >> >>>>>
> >> >>>>> No functionality change.
> >> >>>>>
> >> >>>>
> >> >>>> Nothing to test! :)
> >> >>>>
> >> >>>>
> >> >>>>
> >> >>>> Eric Christopher wrote:
> >> >>>>
> >> >>>>>
> >> >>>>> Testcase? :)
> >> >>>>>
> >> >>>>> -eric
> >> >>>>>
> >> >>>>> On Sat, Oct 5, 2013 at 11:19 AM, Richard
> >> >>>>> Mitton<richard at codersnotes.com>
> >> >>>>> wrote:
> >> >>>>>
> >> >>>>>
> >> >>>>>>
> >> >>>>>> Formally added an explicit enum for DWARF TLS support. No
> >> >>>>>> functionality
> >> >>>>>> change.
> >> >>>>>>
> >> >>>>>>
> >> >>>>>> http://llvm-reviews.chandlerc.com/D1845
> >> >>>>>>
> >> >>>>>> Files:
> >> >>>>>>     include/llvm/Support/Dwarf.h
> >> >>>>>>     lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> >> >>>>>>     lib/Support/Dwarf.cpp
> >> >>>>>>
> >> >>>>>> Index: include/llvm/Support/Dwarf.h
> >> >>>>>>
> ===================================================================
> >> >>>>>> --- include/llvm/Support/Dwarf.h
> >> >>>>>> +++ include/llvm/Support/Dwarf.h
> >> >>>>>> @@ -486,6 +486,9 @@
> >> >>>>>>      DW_OP_lo_user = 0xe0,
> >> >>>>>>      DW_OP_hi_user = 0xff,
> >> >>>>>>
> >> >>>>>> +  // Extensions for GNU-style thread-local storage.
> >> >>>>>> +  DW_OP_GNU_push_tls_address = 0xe0,
> >> >>>>>> +
> >> >>>>>>      // Extensions for Fission proposal.
> >> >>>>>>      DW_OP_GNU_addr_index = 0xfb,
> >> >>>>>>      DW_OP_GNU_const_index = 0xfc,
> >> >>>>>> Index: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> >> >>>>>>
> ===================================================================
> >> >>>>>> --- lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> >> >>>>>> +++ lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> >> >>>>>> @@ -1513,14 +1513,15 @@
> >> >>>>>>            // 1) Start with a constNu of the appropriate pointer
> >> >>>>>> size
> >> >>>>>>            addUInt(Block, 0, dwarf::DW_FORM_data1,
> >> >>>>>>                    PointerSize == 4 ? dwarf::DW_OP_const4u :
> >> >>>>>> dwarf::DW_OP_const8u);
> >> >>>>>> -        // 2) containing the (relocated) address of the TLS
> >> >>>>>> variable
> >> >>>>>> +        // 2) containing the (relocated) offset of the TLS
> >> >>>>>> variable
> >> >>>>>> +        //    within the module's TLS block.
> >> >>>>>>            addExpr(Block, 0, dwarf::DW_FORM_udata, Expr);
> >> >>>>>>          } else {
> >> >>>>>>            addUInt(Block, 0, dwarf::DW_FORM_data1,
> >> >>>>>> dwarf::DW_OP_GNU_const_index);
> >> >>>>>>            addUInt(Block, 0, dwarf::DW_FORM_udata,
> >> >>>>>> DU->getAddrPoolIndex(Expr));
> >> >>>>>>          }
> >> >>>>>> -      // 3) followed by a custom OP to tell the debugger about
> TLS
> >> >>>>>> (presumably)
> >> >>>>>> -      addUInt(Block, 0, dwarf::DW_FORM_data1,
> >> >>>>>> dwarf::DW_OP_lo_user);
> >> >>>>>> +      // 3) followed by a custom OP to make the debugger do a
> TLS
> >> >>>>>> lookup.
> >> >>>>>> +      addUInt(Block, 0, dwarf::DW_FORM_data1,
> >> >>>>>> dwarf::DW_OP_GNU_push_tls_address);
> >> >>>>>>        } else
> >> >>>>>>          addOpAddress(Block, Sym);
> >> >>>>>>        // Do not create specification DIE if context is either
> >> >>>>>> compile
> >> >>>>>> unit
> >> >>>>>> Index: lib/Support/Dwarf.cpp
> >> >>>>>>
> ===================================================================
> >> >>>>>> --- lib/Support/Dwarf.cpp
> >> >>>>>> +++ lib/Support/Dwarf.cpp
> >> >>>>>> @@ -456,10 +456,11 @@
> >> >>>>>>      case DW_OP_bit_piece:                  return
> >> >>>>>> "DW_OP_bit_piece";
> >> >>>>>>      case DW_OP_implicit_value:             return
> >> >>>>>> "DW_OP_implicit_value";
> >> >>>>>>      case DW_OP_stack_value:                return
> >> >>>>>> "DW_OP_stack_value";
> >> >>>>>> -  case DW_OP_lo_user:                    return "DW_OP_lo_user";
> >> >>>>>> -  case DW_OP_hi_user:                    return "DW_OP_hi_user";
> >> >>>>>>
> >> >>>>>> -    // DWARF5 Fission Proposal Op Extensions
> >> >>>>>> +  // GNU thread-local storage
> >> >>>>>> +  case DW_OP_GNU_push_tls_address:       return
> >> >>>>>> "DW_OP_GNU_push_tls_address";
> >> >>>>>> +
> >> >>>>>> +  // DWARF5 Fission Proposal Op Extensions
> >> >>>>>>      case DW_OP_GNU_addr_index:             return
> >> >>>>>> "DW_OP_GNU_addr_index";
> >> >>>>>>      case DW_OP_GNU_const_index:            return
> >> >>>>>> "DW_OP_GNU_const_index";
> >> >>>>>>      }
> >> >>>>>>
> >> >>>>>> _______________________________________________
> >> >>>>>> llvm-commits mailing list
> >> >>>>>> llvm-commits at cs.uiuc.edu
> >> >>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >> >>>>>>
> >> >>>>>>
> >> >>>>>>
> >> >>>>
> >> >>>> _______________________________________________
> >> >>>> llvm-commits mailing list
> >> >>>> llvm-commits at cs.uiuc.edu
> >> >>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >> >>>>
> >> >>
> >> >> _______________________________________________
> >> >> llvm-commits mailing list
> >> >> llvm-commits at cs.uiuc.edu
> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >> >
> >> >
> >> >
> >> > _______________________________________________
> >> > llvm-commits mailing list
> >> > llvm-commits at cs.uiuc.edu
> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131007/b731f9ec/attachment.html>


More information about the llvm-commits mailing list