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

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added inline comments.
This revision is now accepted and ready to land.


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


https://reviews.llvm.org/D37725