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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
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:
> pcc wrote:
> > 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?
> By "this" I mean the functionality committed with/supposedly verified by this test:
> 
> "DebugInfo: Produce debug_{gnu_}pub{names,types} entries when explicitly requested, even in -gmlt or when empty"
> 
> Though admittedly I was mostly looking at gnu pubnames there, not sure if gold uses non-gnu pubnames for gdb-index or not (I think not). So maybe the original generalization over gnu and non-gnu pubnames isn't important (in which case removing this RUN line and only testing the gnu case would be fine), but it does seem a bit inconsistenty, I guess.
Okay, looks like that is being tested for by the first line. I'll go ahead and remove the second line, we can always bring it back if we add a supported way of doing gmlt + non-gnu pubnames.


https://reviews.llvm.org/D37725