[Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for IR input

[MaskRay]

Summary

This generalizes D94647 to IR input, as suggested by @tejohnson.
Ideally the driver should just forward split dwarf options, but doing this currently will cause clang -gsplit-dwarf -c a.c to create a .dwo with just .strtab.

[tejohnson]

lgtm

[dblaikie]

Is there any way to condition this on the type of the output, rather than the input? (or, more specifically, on whether machine code is being generated)

Or maybe we could always pass the split-dwarf-file down through LLVM and not need to conditionalize it at all? It'd be a no-op if there's no DWARF in the IR anyway?

[MaskRay]

In D94655#2498504, @dblaikie wrote:

Is there any way to condition this on the type of the output, rather than the input? (or, more specifically, on whether machine code is being generated)

Or maybe we could always pass the split-dwarf-file down through LLVM and not need to conditionalize it at all? It'd be a no-op if there's no DWARF in the IR anyway?

I tried replacing if (IRInput || Args.hasArg(options::OPT_g_Group)) { with if (1), -gsplit-dwarf may produce .dwo for regular non-g .c compile.

Since we already have the

// For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit more
// complicated if you've disabled inline info in the skeleton CUs
// (SplitDWARFInlining) - then there's value in composing split-dwarf and
// line-tables-only, so let those compose naturally in that case.

logic, I think altering DwarfFission in the driver is fine. If not, I'd hope the backend to process DwarfFission ...

[dblaikie]

In D94655#2498548, @MaskRay wrote:

In D94655#2498504, @dblaikie wrote:

Is there any way to condition this on the type of the output, rather than the input? (or, more specifically, on whether machine code is being generated)

Or maybe we could always pass the split-dwarf-file down through LLVM and not need to conditionalize it at all? It'd be a no-op if there's no DWARF in the IR anyway?

I tried replacing if (IRInput || Args.hasArg(options::OPT_g_Group)) { with if (1), -gsplit-dwarf may produce .dwo for regular non-g .c compile.

Are you saying that if you make that change -gsplit-dwarf does cause .dwo files to be created for non-g .c compiles? Do the dwo files have anything in them? I had modified llvm to dynamically choose split or non-split based on whether there was enough data to be worth splitting into a .dwo file, but I guess that situation might still be producing an empty .dwo file which isn't ideal - I haven't tested that.

Since we already have the

// For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit more
// complicated if you've disabled inline info in the skeleton CUs
// (SplitDWARFInlining) - then there's value in composing split-dwarf and
// line-tables-only, so let those compose naturally in that case.

logic, I think altering DwarfFission in the driver is fine. If not, I'd hope the backend to process DwarfFission ...

Sorry, I'm not understanding this comment - could you describe/rephrase it in more detail?

[MaskRay]

In D94655#2498669, @dblaikie wrote:

In D94655#2498548, @MaskRay wrote:

In D94655#2498504, @dblaikie wrote:

Is there any way to condition this on the type of the output, rather than the input? (or, more specifically, on whether machine code is being generated)

Or maybe we could always pass the split-dwarf-file down through LLVM and not need to conditionalize it at all? It'd be a no-op if there's no DWARF in the IR anyway?

I tried replacing if (IRInput || Args.hasArg(options::OPT_g_Group)) { with if (1), -gsplit-dwarf may produce .dwo for regular non-g .c compile.

Are you saying that if you make that change -gsplit-dwarf does cause .dwo files to be created for non-g .c compiles? Do the dwo files have anything in them? I had modified llvm to dynamically choose split or non-split based on whether there was enough data to be worth splitting into a .dwo file, but I guess that situation might still be producing an empty .dwo file which isn't ideal - I haven't tested that.

% clang a.c -gsplit-dwarf -c
% readelf -WS a.dwo                
There are 2 section headers, starting at offset 0x50:

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .strtab           STRTAB          0000000000000000 000040 000009 00      0   0  1

Since we already have the

// For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit more
// complicated if you've disabled inline info in the skeleton CUs
// (SplitDWARFInlining) - then there's value in composing split-dwarf and
// line-tables-only, so let those compose naturally in that case.

logic, I think altering DwarfFission in the driver is fine. If not, I'd hope the backend to process DwarfFission ...

Sorry, I'm not understanding this comment - could you describe/rephrase it in more detail?

The driver already decides that DwarfFission should be disabled in -g0 and -g1 cases. If the driver does not already do this, passing through DwarfFission in the driver and letting CC1 handle it seems right to me.

Since the driver already has some logic, I think adding more logic about IR input types is fine.

[dblaikie]

In D94655#2498811, @MaskRay wrote:

In D94655#2498669, @dblaikie wrote:

In D94655#2498548, @MaskRay wrote:

In D94655#2498504, @dblaikie wrote:

Is there any way to condition this on the type of the output, rather than the input? (or, more specifically, on whether machine code is being generated)

Or maybe we could always pass the split-dwarf-file down through LLVM and not need to conditionalize it at all? It'd be a no-op if there's no DWARF in the IR anyway?

I tried replacing if (IRInput || Args.hasArg(options::OPT_g_Group)) { with if (1), -gsplit-dwarf may produce .dwo for regular non-g .c compile.

Are you saying that if you make that change -gsplit-dwarf does cause .dwo files to be created for non-g .c compiles? Do the dwo files have anything in them? I had modified llvm to dynamically choose split or non-split based on whether there was enough data to be worth splitting into a .dwo file, but I guess that situation might still be producing an empty .dwo file which isn't ideal - I haven't tested that.

% clang a.c -gsplit-dwarf -c
% readelf -WS a.dwo                
There are 2 section headers, starting at offset 0x50:

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .strtab           STRTAB          0000000000000000 000040 000009 00      0   0  1

OK, thanks. Yeah, wouldn't mind if that got fixed at some point. It turns up in existing behavior/without patches with situations like clang++ -gmlt -gsplit-dwarf x.cpp -c if there's no inlining (that's the feature I mentioned/referenced in my previous comment) - since the split CU would be empty, LLVM avoids emitting a split CU at all - but does end up with that empty dwo situation you described. If that bug were fixed (lazily choosing not to create a dwo file at all, if no CUs were emitted into it) then we could pass the split dwarf request straight down unmodified all the time - making the architecture as orthogonal as the user-facing feature is intended to be.

Since we already have the

// For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit more
// complicated if you've disabled inline info in the skeleton CUs
// (SplitDWARFInlining) - then there's value in composing split-dwarf and
// line-tables-only, so let those compose naturally in that case.

logic, I think altering DwarfFission in the driver is fine. If not, I'd hope the backend to process DwarfFission ...

Sorry, I'm not understanding this comment - could you describe/rephrase it in more detail?

The driver already decides that DwarfFission should be disabled in -g0 and -g1 cases. If the driver does not already do this, passing through DwarfFission in the driver and letting CC1 handle it seems right to me.

Since the driver already has some logic, I think adding more logic about IR input types is fine.

My concern is that we'll miss other cases where code generation/dwo file creation is done when handling special cases like this & the code would be more robust if we didn't have to special case things like this.

But I guess it'll do for now.

clang/lib/Driver/ToolChains/Clang.cpp

static DwarfFissionKind getDebugFissionKind(const Driver &D,
   if (Value == "single")
     return DwarfFissionKind::Single;
 
   D.Diag(diag::err_drv_unsupported_option_argument)
       << Arg->getOption().getName() << Arg->getValue();
   return DwarfFissionKind::None;
 }
 
-static void RenderDebugOptions(const ToolChain &TC, const Driver &D,
+static void renderDebugOptions(const ToolChain &TC, const Driver &D,
                                const llvm::Triple &T, const ArgList &Args,
-                               bool EmitCodeView, ArgStringList &CmdArgs,
+                               bool EmitCodeView, bool IRInput,
+                               ArgStringList &CmdArgs,
                                codegenoptions::DebugInfoKind &DebugInfoKind,
                                DwarfFissionKind &DwarfFission) {
   if (Args.hasFlag(options::OPT_fdebug_info_for_profiling,
                    options::OPT_fno_debug_info_for_profiling, false) &&
       checkDebugInfoOption(
           Args.getLastArg(options::OPT_fdebug_info_for_profiling), Args, D, TC))
     CmdArgs.push_back("-fdebug-info-for-profiling");
 
   // The 'g' groups options involve a somewhat intricate sequence of decisions
   // about what to pass from the driver to the frontend, but by the time they
   // reach cc1 they've been factored into three well-defined orthogonal choices:
   //  * what level of debug info to generate
   //  * what dwarf version to write
   //  * what debugger tuning to use
   // This avoids having to monkey around further in cc1 other than to disable
   // codeview if not running in a Windows environment. Perhaps even that
   // decision should be made in the driver as well though.
   llvm::DebuggerKind DebuggerTuning = TC.getDefaultDebuggerTuning();
 
   bool SplitDWARFInlining =
       Args.hasFlag(options::OPT_fsplit_dwarf_inlining,
                    options::OPT_fno_split_dwarf_inlining, false);
 
-  // Normally -gsplit-dwarf is only useful with -gN. For -gsplit-dwarf in the
-  // backend phase of a distributed ThinLTO which does object file generation
-  // and no IR generation, -gN should not be needed. So allow -gsplit-dwarf with
-  // either -gN or -fthinlto-index=.
-  if (Args.hasArg(options::OPT_g_Group) ||
-      Args.hasArg(options::OPT_fthinlto_index_EQ)) {
+  // Normally -gsplit-dwarf is only useful with -gN. For IR input, Clang does
+  // object file generation and no IR generation, -gN should not be needed. So
+  // allow -gsplit-dwarf with either -gN or IR input.
+  if (IRInput || Args.hasArg(options::OPT_g_Group)) {
     Arg *SplitDWARFArg;
     DwarfFission = getDebugFissionKind(D, Args, SplitDWARFArg);
     if (DwarfFission != DwarfFissionKind::None &&
         !checkDebugInfoOption(SplitDWARFArg, Args, D, TC)) {
       DwarfFission = DwarfFissionKind::None;
       SplitDWARFInlining = false;
     }
   }
#endif
   bool EmitCodeView = false;
 
   // Add clang-cl arguments.
   types::ID InputType = Input.getType();
   if (D.IsCLMode())
     AddClangCLArgs(Args, InputType, CmdArgs, &DebugInfoKind, &EmitCodeView);
 
   DwarfFissionKind DwarfFission = DwarfFissionKind::None;
-  RenderDebugOptions(TC, D, RawTriple, Args, EmitCodeView, CmdArgs,
-                     DebugInfoKind, DwarfFission);
+  renderDebugOptions(TC, D, RawTriple, Args, EmitCodeView,
+                     types::isLLVMIR(InputType), CmdArgs, DebugInfoKind,
+                     DwarfFission);
 
   // Add the split debug info name to the command lines here so we
   // can propagate it to the backend.
   bool SplitDWARF = (DwarfFission != DwarfFissionKind::None) &&
                     (TC.getTriple().isOSBinFormatELF() ||
                      TC.getTriple().isOSBinFormatWasm()) &&
                     (isa<AssembleJobAction>(JA) || isa<CompileJobAction>(JA) ||
                      isa<BackendJobAction>(JA));

clang/test/Driver/split-debug.c

 // DARWIN:     "-debug-info-kind=standalone"
 // DARWIN-NOT: "-split-dwarf
 
 /// -gsplit-dwarf is a no-op if no -g is specified.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefix=G0
 
 /// ... unless -fthinlto-index= is specified.
 // RUN: echo > %t.bc
-// RUN: %clang -### -c -target x86_64 -fthinlto-index=dummy -gsplit-dwarf %t.bc 2>&1 | FileCheck %s --check-prefix=THINLTO
+// RUN: %clang -### -c -target x86_64 -fthinlto-index=dummy -gsplit-dwarf %t.bc 2>&1 | FileCheck %s --check-prefix=IR
+// RUN: %clang -### -c -target x86_64 -gsplit-dwarf -x ir %t.bc 2>&1 | FileCheck %s --check-prefix=IR
 
-// THINLTO-NOT:  "-debug-info-kind=
-// THINLTO:      "-ggnu-pubnames"
-// THINLTO-SAME: "-split-dwarf-file" "{{.*}}.dwo" "-split-dwarf-output" "{{.*}}.dwo"
+// IR-NOT:  "-debug-info-kind=
+// IR:      "-ggnu-pubnames"
+// IR-SAME: "-split-dwarf-file" "{{.*}}.dwo" "-split-dwarf-output" "{{.*}}.dwo"
 
 /// -gno-split-dwarf disables debug fission.
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf -g -gno-split-dwarf %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g -gno-split-dwarf %s 2>&1 | FileCheck %s --check-prefix=NOSPLIT
 // RUN: %clang -### -c -target x86_64 -gno-split-dwarf -g -gsplit-dwarf %s 2>&1 | FileCheck %s --check-prefixes=NOINLINE,SPLIT
 
 // NOSPLIT:     "-debug-info-kind=limited"
 // NOSPLIT-NOT: "-ggnu-pubnames"