[PATCH] D87011: [DebugInfo] Add the -dwarf64 switch to llc and other internal tools (4/19).

Thu Sep 3 12:00:32 PDT 2020

dblaikie added inline comments.

================
Comment at: llvm/test/DebugInfo/X86/dwarf64-support.ll:6
+; should be ignored for earlier versions.
+; RUN: llc -mtriple=x86_64 -dwarf-version=2 -dwarf64 -filetype=obj %s -o - | \
+; RUN:   llvm-dwarfdump -debug-line - | \
----------------
MaskRay wrote:
> ikudrin wrote:
> > MaskRay wrote:
> > > If `-dwarf-version=2 -dwarf64` does not make sense, shouldn't the combo be errored to prevent misuse?
> > I am not sure where to add that check and reporting. It looks like for internal tools erroneous combinations are just ignored. For example, for NVPTX, setting the DWARF version is silently ignored (see lines 372-374 in `DwarfDebug.cpp`). Thus, my change just follows the crowd.
> I guess NVPTX does so for quick MVP prototype ("[DEBUG] Initial adaptation of NVPTX target for debug info emission."). Downgrading DWARF version this way allows them to use -gdwarf-* quickly with their auxiliary target triples (`"-triple" "x86_64-unknown-linux-gnu" "-aux-triple" "nvptx64-nvidia-cuda"`)
> 
> For clang -gdwarf-4, I think it probably does not hurt when an auxiliary target triple does support DWARF v4 and downgrades to v2. For the testing tool llc, we probably should emit better diagnostic to remind the user.
Eh, there are a fair few flags I think at the llc level we don't bother to provide error messages about. One that comes to mind would be type units - the flag silently does nothing on non-ELF targets, for instance. Enabling debug-macro does nothing when using v4 Split DWARF.

Might be nice, but I wouldn't say it's necessary - llc's a tool for us to test LLVM with & some flags have no effect in certain circumstances. (heck, even at the clang driver level that's true - lots of flags are just no-ops in certain situations, rather than providing error messages about their incompatibility)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87011/new/

https://reviews.llvm.org/D87011