Page MenuHomePhabricator

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

Authored by MaskRay on Wed, Jan 13, 11:39 PM.

Details

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.

Diff Detail

Event Timeline

MaskRay requested review of this revision.Wed, Jan 13, 11:39 PM
MaskRay created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptWed, Jan 13, 11:39 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
This revision is now accepted and ready to land.Thu, Jan 14, 8:28 AM

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?

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

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?

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 accepted this revision.Thu, Jan 14, 11:24 AM

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.

MaskRay edited the summary of this revision. (Show Details)Thu, Jan 14, 11:44 AM