[PATCH] D37725: Remove -generate-dwarf-pub-sections flag.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: llvm/test/DebugInfo/Generic/dwarf-public-names.ll:3
 
-; RUN: %llc_dwarf -generate-dwarf-pub-sections=Enable -filetype=obj -o %t.o < %s
 ; RUN: llvm-dwarfdump -debug-dump=pubnames %t.o | FileCheck %s
----------------
dblaikie wrote:
> I think it'd be better to change the IR to enable gnu-pubnames here (& in other tests where this patch is currently adding -debugger-tune) - makes it more explicit what the purpose is (I mean, yeah, it may get lost in the noise of the debug info metadata... but still)
I think this test (and the others like it) are intended to cover non-GNU pubnames, though.


================
Comment at: llvm/test/DebugInfo/X86/gnu-public-names-gmlt.ll:2
 ; 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
+; RUN: sed -e 's/emissionKind: LineTablesOnly/emissionKind: FullDebug/' %s | llc -mtriple=x86_64-pc-linux-gnu -filetype=obj | llvm-dwarfdump - | FileCheck --check-prefix=PUB --check-prefix=CHECK %s
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -filetype=obj < %s | llvm-dwarfdump - | FileCheck --check-prefix=NONE %s
----------------
dblaikie wrote:
> What's this for? Maybe this removes the purpose of this test (it seems to be testing gnu-public-names and gmlt, which is now impossible). Perhaps the test should be deleted?
> 
> Though looking at the original commit (r303894) that added this test, maybe there's something to be concerned about that either this patch or the previous one may've regressed.
> 
> Probably worth checking if this does work or did work when it was committed, I guess.
> it seems to be testing gnu-public-names and gmlt, which is now impossible

Actually, I think the change makes non-GNU public-names + gmlt impossible. 

> Perhaps the test should be deleted?

Removing this line seems fine to me. If the purpose of this test is to test things related to gmlt I guess it wouldn't make sense to disable gmlt.

> Probably worth checking if this does work or did work when it was committed, I guess.

It isn't clear to me what you mean by "this". Do you mean whether gold accepts the object file?


https://reviews.llvm.org/D37725