[PATCH] D37655: IR: Represent -ggnu-pubnames with a flag on the DICompileUnit.

[Peter Collingbourne via llvm-commits]

On Mon, Sep 11, 2017 at 4:46 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Mon, Sep 11, 2017 at 4:42 PM Peter Collingbourne via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> pcc added a comment.
>>
>> In https://reviews.llvm.org/D37655#867084, @dblaikie wrote:
>>
>> > Reckon maybe a test case with two units - one using gnu pubnames, one
>> not, would be worth having to demonstrate that all works fine & is
>> appropriately respected?
>>
>>
>> Added, see `llvm/test/DebugInfo/X86/gnu-public-names-multiple-cus.ll`.
>>
>>
>>
>> ================
>> Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:412-422
>> +bool DwarfDebug::hasDwarfPubSections(DwarfCompileUnit &CU) const {
>>    // Opting in to GNU Pubnames/types overrides the default to ensure
>> these are
>>    // generated for things like Gold's gdb_index generation.
>> -  if (GenerateGnuPubSections)
>> +  if (CU.getCUNode()->getGnuPubnames())
>>      return true;
>>
>>    if (DwarfPubSections == Default)
>> ----------------
>> dblaikie wrote:
>> > Looks like this function could be moved to be a member of
>> DwarfCompileUnit? (& move the DwarfPubSections opt into the
>> DwarfCompileUnit.cpp file?)
>> >
>> > Just a thought - see how you feel about it, if you think it's better
>> here, that's fine.
>> I think that makes sense, done.
>>
>> What do you think about removing the `-generate-dwarf-pub-sections` flag
>> entirely? It seems that we only use it in a handful of tests where it looks
>> like it would suffice to use `-debugger-tune` instead. In any event, that
>> can probably be done as a followup.
>>
>
> If the sed-based testing seems to suffice, sure - seems OK to remove.
>

I've sent a followup patch https://reviews.llvm.org/D37725 which removes
the flag.

Peter

>
>
>>
>>
>> ================
>> Comment at: llvm/test/DebugInfo/X86/gnu-public-names-gmlt.ll:1
>> -; RUN: llc -mtriple=x86_64-pc-linux-gnu -filetype=obj < %s
>> -generate-gnu-dwarf-pub-sections | llvm-dwarfdump - | FileCheck
>> --check-prefix=GPUB --check-prefix=CHECK %s
>> +; RUN: sed -e 's/gnuPubnames: false/gnuPubnames: true/' %s | llc
>> -mtriple=x86_64-pc-linux-gnu -filetype=obj | llvm-dwarfdump - | FileCheck
>> --check-prefix=GPUB --check-prefix=CHECK %s
>>  ; RUN: llc -mtriple=x86_64-pc-linux-gnu -filetype=obj < %s
>> -generate-dwarf-pub-sections=Enable | llvm-dwarfdump - | FileCheck
>> --check-prefix=PUB --check-prefix=CHECK %s
>> ----------------
>> dblaikie wrote:
>> > Is this use of 'sed' in a RUN line portable to windows (I forget
>> exactly which command line tools are available/dependendable in the lit
>> environment there)?
>> I think so, we are using sed like this in a few places already:
>> http://llvm-cs.pcc.me.uk/?q=RUN%3A+sed
>>
>>
>> https://reviews.llvm.org/D37655
>>
>>
>>
>>


-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170911/fb2b95a6/attachment.html>