Page MenuHomePhabricator

[DebugInfo] Combine Trivial and NonTrivial flags
ClosedPublic

Authored by asmith on Mar 13 2019, 10:25 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

asmith created this revision.Mar 13 2019, 10:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 13 2019, 10:25 PM
asmith updated this revision to Diff 190705.Mar 14 2019, 12:32 PM

Updated the C API to match

asmith updated this revision to Diff 194252.Apr 8 2019, 11:50 PM
dblaikie added inline comments.
include/llvm/IR/DebugInfoFlags.def
57–60 ↗(On Diff #194252)

@aprantl - could you review this for bitcode compatibility?

I imagine reusing a bit for a different meaning (the exact opposite) would be problematic?

Would we have to settle for keeping the bitcode encoding as 1=trivial 0=nontrivial (only for structs/classes? Maybe enums too?) - or burn the 26th bit and keep the 30th bit as NonTrivial?

aprantl added inline comments.Apr 10 2019, 2:43 PM
include/llvm/IR/DebugInfoFlags.def
57–60 ↗(On Diff #194252)

Since this only affects PDB debug info I personally don't worry about breaking bitcode compatibility, however other people may do so.

The correct way to update this is to bump the version of the records (DICompositeType) and parse the old or the new format accordingly.

If this isn't done any LLVM module with bitcode that currently uses bit 30 (which is now > Largest) will cause a verifier error and the debug info from that module will be dropped with a warning. LTO users may or may not care about this.

If the NonTrivial flag was not present in the last open source release and none of the Windows stakeholders object, I think we can land this as a bugfix, ignoring bitcode compatibility.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 11 2019, 1:23 PM
Closed by commit rL358220: [DebugInfo] Combine Trivial and NonTrivial flags (authored by asmith, committed by ). · Explain Why
This revision was automatically updated to reflect the committed changes.

Confirming the nontrivial flag was not in the llvm 8 release.