[PATCH] Minor cleanup for GNU TLS support

[Eric Christopher]

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" :)

-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
>> >
>
>