[DebugInfo] IR/Bitcode changes for DISubprogram flags

[probinson]

Summary

Packing the flags into one bitcode word will simplify adding new flags in the future.

[probinson]

Ping.

[aprantl]

I'm assuming we already have plenty of DISubprograms with flags in the old format as .bc tests that cover the bitcode upgrade?

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

1645

Am I right in assuming that these dead fields take up virtually no extra space in the bitcode? Otherwise we might want to add an extra bit distinguish old vs. a newer, more compact record layout.

llvm/lib/IR/DebugInfoMetadata.cpp

554

nit: comment should go on the line before.

[probinson]

In D54755#1308216, @aprantl wrote:

I'm assuming we already have plenty of DISubprograms with flags in the old format as .bc tests that cover the bitcode upgrade?

That was my thinking, but perhaps it would do no harm to add another specifically for this. Basically it would be a pre-patch Assembler/disubprogram.ll run through a pre-patch llvm-as which the test would run through llvm-dis and verify the flags came out right.

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

1645

Each field in Record is 64-bit, so as written this is wasting 24 bytes per DISubprogram in the bitcode. Compressing those out would mean the reader would pretty much have to handle the old/new formats separately, as the indexes of all fields after this one would vary too much to handle cleanly any other way.
I'm happy to make that change if you think the size is worthwhile.

[dblaikie]

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

1645

I don't /think/ that's how the bitcode format works, though, right? I believe it's a more compact encoding than using all 64 bits regardless of the value encoded... but I don't know the specifics/not very sure about it.

[dexonsmith]

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

1645

@dblaikie should be right. IIRC the default (in absence of an explicit abbreviation) is vbr-6, so that's 6 bits times three for 18 bits.

That said, I'm not convinced this would be hard to deal with in the reader. There's probably a way to reorganize the logic to avoid forking.

[probinson]

Don't emit unused fields to the bitcode. Accommodate the new variant in the reader.

This does not address all comments, but I wanted to show @dexonsmith the change for bitcode format that he asked for.

[aprantl]

I think it would be nice to have one small explicit "DISubprogram version 4" .bc upgrade test, otherwise this LGTM now. Thanks!

[probinson]

In D54755#1311498, @aprantl wrote:

I think it would be nice to have one small explicit "DISubprogram version 4" .bc upgrade test, otherwise this LGTM now. Thanks!

In plan. Also pls look at Clang test updates in D54756.

[dexonsmith]

In D54755#1311526, @probinson wrote:

In D54755#1311498, @aprantl wrote:

I think it would be nice to have one small explicit "DISubprogram version 4" .bc upgrade test, otherwise this LGTM now. Thanks!

In plan. Also pls look at Clang test updates in D54756.

Nice!

[dexonsmith]

In D54755#1311896, @dexonsmith wrote:

Nice!

Oops, I had meant to respond to this one:

This does not address all comments, but I wanted to show @dexonsmith the change for bitcode format that he asked for.

[aprantl]

While I was updating some internal testcases I wondered: why is DIFlagPrototype not a DISPFlag? Does this flag make sense on something other than functions?

[probinson]

In D54755#1312971, @aprantl wrote:

While I was updating some internal testcases I wondered: why is DIFlagPrototype not a DISPFlag? Does this flag make sense on something other than functions?

Because I was not interested in moving existing flags from the old word to the new word in the first round. There are others that probably apply only to subprograms:
NoReturn, MainSubprogram; probably Thunk, All CallsDescribed; maybe Trivial, Explicit.
Really we should do some kind of audit, and do a bulk move in one go so we can minimize the bitcode-upgrade pain.

[probinson]

In D54755#1311897, @dexonsmith wrote:

Nice!

Thanks--by clever choice of where to place the new field, it was possible to minimize the variants. I'm not super happy about having to track two different offsets, but it's not much worse than the old code.

[probinson]

In D54755#1313064, @probinson wrote:

In D54755#1312971, @aprantl wrote:

While I was updating some internal testcases I wondered: why is DIFlagPrototype not a DISPFlag? Does this flag make sense on something other than functions?

Because I was not interested in moving existing flags from the old word to the new word in the first round. There are others that probably apply only to subprograms:
NoReturn, MainSubprogram; probably Thunk, All CallsDescribed; maybe Trivial, Explicit.
Really we should do some kind of audit, and do a bulk move in one go so we can minimize the bitcode-upgrade pain.

One additional problem is that the existing flags are exposed as part of the public API, i.e. to front-ends, so we'll need to work out how to avoid Bad Stuff(tm) if we repurpose any existing flag bits.

[dexonsmith]

In D54755#1313098, @probinson wrote:

In D54755#1313064, @probinson wrote:

In D54755#1312971, @aprantl wrote:

While I was updating some internal testcases I wondered: why is DIFlagPrototype not a DISPFlag? Does this flag make sense on something other than functions?

Because I was not interested in moving existing flags from the old word to the new word in the first round. There are others that probably apply only to subprograms:
NoReturn, MainSubprogram; probably Thunk, All CallsDescribed; maybe Trivial, Explicit.
Really we should do some kind of audit, and do a bulk move in one go so we can minimize the bitcode-upgrade pain.

One additional problem is that the existing flags are exposed as part of the public API, i.e. to front-ends, so we'll need to work out how to avoid Bad Stuff(tm) if we repurpose any existing flag bits.

Another option here is not to merge them at all in bitcode (I didn't consider this before). You can add an abbreviation for a fixed-width field of 1 bit each:
https://llvm.org/docs/BitCodeFormat.html#fixed-width-value

[probinson]

In D54755#1313679, @dexonsmith wrote:

In D54755#1313098, @probinson wrote:

In D54755#1313064, @probinson wrote:

In D54755#1312971, @aprantl wrote:

While I was updating some internal testcases I wondered: why is DIFlagPrototype not a DISPFlag? Does this flag make sense on something other than functions?

Because I was not interested in moving existing flags from the old word to the new word in the first round. There are others that probably apply only to subprograms:
NoReturn, MainSubprogram; probably Thunk, All CallsDescribed; maybe Trivial, Explicit.
Really we should do some kind of audit, and do a bulk move in one go so we can minimize the bitcode-upgrade pain.

One additional problem is that the existing flags are exposed as part of the public API, i.e. to front-ends, so we'll need to work out how to avoid Bad Stuff(tm) if we repurpose any existing flag bits.

Another option here is not to merge them at all in bitcode (I didn't consider this before). You can add an abbreviation for a fixed-width field of 1 bit each:
https://llvm.org/docs/BitCodeFormat.html#fixed-width-value

Wouldn't that cause every new flag to be a bitcode format change? The value of having a flags word is that new flags in the same word don't affect the bitcode format.

llvm/include/llvm/IR/DebugInfoFlags.def

 #undef DI_FLAG_LARGEST_NEEDED
 #endif
 
 // Subprogram-specific flags kept in DISubprogram.
 
 // Use this as a zero/initialization value.
 // For example: void foo(DISPFlags Flags = SPFlagZero).
 HANDLE_DISP_FLAG(0, Zero)
-// Virtuality is a two-bit valued field.
-HANDLE_DISP_FLAG(0u, Nonvirtual)
+// Virtuality is a two-bit enum field in the LSB of the word.
+// Values should match DW_VIRTUALITY_*.
 HANDLE_DISP_FLAG(1u, Virtual)
 HANDLE_DISP_FLAG(2u, PureVirtual)
 HANDLE_DISP_FLAG((1u << 2), LocalToUnit)
 HANDLE_DISP_FLAG((1u << 3), Definition)
 HANDLE_DISP_FLAG((1u << 4), Optimized)
 
 #ifdef DISP_FLAG_LARGEST_NEEDED
 // Intended to be used with ADT/BitmaskEnum.h.
 // NOTE: Always must be equal to largest flag, check this when adding new flags.
 HANDLE_DISP_FLAG((1 << 4), Largest)
 #undef DISP_FLAG_LARGEST_NEEDED
 #endif
 
 #undef HANDLE_DI_FLAG
 #undef HANDLE_DISP_FLAG

llvm/include/llvm/IR/DebugInfoMetadata.h

class DISubprogram : public DILocalScope {
   int ThisAdjustment;
 
 public:
   /// Debug info subprogram flags.
   enum DISPFlags : uint32_t {
 #define HANDLE_DISP_FLAG(ID, NAME) SPFlag##NAME = ID,
 #define DISP_FLAG_LARGEST_NEEDED
 #include "llvm/IR/DebugInfoFlags.def"
-    SPFlagVirtuality = SPFlagNonvirtual | SPFlagVirtual | SPFlagPureVirtual,
+    SPFlagNonvirtual = SPFlagZero,
+    SPFlagVirtuality = SPFlagVirtual | SPFlagPureVirtual,
     LLVM_MARK_AS_BITMASK_ENUM(SPFlagLargest)
   };
+
+  static DISPFlags getFlag(StringRef Flag);
+  static StringRef getFlagString(DISPFlags Flag);
+
+  /// Split up a flags bitfield for easier printing.
+  ///
+  /// Split \c Flags into \c SplitFlags, a vector of its components.  Returns
+  /// any remaining (unrecognized) bits.
+  static DISPFlags splitFlags(DISPFlags Flags,
+                              SmallVectorImpl<DISPFlags> &SplitFlags);
+
   // Helper for converting old bitfields to new flags word.
   static DISPFlags toSPFlags(bool IsLocalToUnit, bool IsDefinition,
                              bool IsOptimized,
                              unsigned Virtuality = SPFlagNonvirtual) {
     // We're assuming virtuality is the low-order field.
     static_assert(
         int(SPFlagVirtual) == int(dwarf::DW_VIRTUALITY_virtual) &&
             int(SPFlagPureVirtual) == int(dwarf::DW_VIRTUALITY_pure_virtual),

llvm/lib/AsmParser/LLLexer.cpp

 
 #undef DWKEYWORD
 
   if (Keyword.startswith("DIFlag")) {
     StrVal.assign(Keyword.begin(), Keyword.end());
     return lltok::DIFlag;
   }
 
+  if (Keyword.startswith("DISPFlag")) {
+    StrVal.assign(Keyword.begin(), Keyword.end());
+    return lltok::DISPFlag;
+  }
+
   if (Keyword.startswith("CSK_")) {
     StrVal.assign(Keyword.begin(), Keyword.end());
     return lltok::ChecksumKind;
   }
 
   if (Keyword == "NoDebug" || Keyword == "FullDebug" ||
       Keyword == "LineTablesOnly" || Keyword == "DebugDirectivesOnly") {
     StrVal.assign(Keyword.begin(), Keyword.end());

llvm/lib/AsmParser/LLParser.cpp

NameTableKindField()
             0, (unsigned)
                    DICompileUnit::DebugNameTableKind::LastDebugNameTableKind) {}
 };
 
 struct DIFlagField : public MDFieldImpl<DINode::DIFlags> {
   DIFlagField() : MDFieldImpl(DINode::FlagZero) {}
 };
 
+struct DISPFlagField : public MDFieldImpl<DISubprogram::DISPFlags> {
+  DISPFlagField() : MDFieldImpl(DISubprogram::SPFlagZero) {}
+};
+
 struct MDSignedField : public MDFieldImpl<int64_t> {
   int64_t Min;
   int64_t Max;
 
   MDSignedField(int64_t Default = 0)
       : ImplTy(Default), Min(INT64_MIN), Max(INT64_MAX) {}
   MDSignedField(int64_t Default, int64_t Min, int64_t Max)
       : ImplTy(Default), Min(Min), Max(Max) {}
if (parseFlag(Val))
       return true;
     Combined |= Val;
   } while (EatIfPresent(lltok::bar));
 
   Result.assign(Combined);
   return false;
 }
 
+/// DISPFlagField
+///  ::= uint32
+///  ::= DISPFlagVector
+///  ::= DISPFlagVector '|' DISPFlag* '|' uint32
+template <>
+bool LLParser::ParseMDField(LocTy Loc, StringRef Name, DISPFlagField &Result) {
+
+  // Parser for a single flag.
+  auto parseFlag = [&](DISubprogram::DISPFlags &Val) {
+    if (Lex.getKind() == lltok::APSInt && !Lex.getAPSIntVal().isSigned()) {
+      uint32_t TempVal = static_cast<uint32_t>(Val);
+      bool Res = ParseUInt32(TempVal);
+      Val = static_cast<DISubprogram::DISPFlags>(TempVal);
+      return Res;
+    }
+
+    if (Lex.getKind() != lltok::DISPFlag)
+      return TokError("expected debug info flag");
+
+    Val = DISubprogram::getFlag(Lex.getStrVal());
+    if (!Val)
+      return TokError(Twine("invalid subprogram debug info flag '") +
+                      Lex.getStrVal() + "'");
+    Lex.Lex();
+    return false;
+  };
+
+  // Parse the flags and combine them together.
+  DISubprogram::DISPFlags Combined = DISubprogram::SPFlagZero;
+  do {
+    DISubprogram::DISPFlags Val;
+    if (parseFlag(Val))
+      return true;
+    Combined |= Val;
+  } while (EatIfPresent(lltok::bar));
+
+  Result.assign(Combined);
+  return false;
+}
+
 template <>
 bool LLParser::ParseMDField(LocTy Loc, StringRef Name,
                             MDSignedField &Result) {
   if (Lex.getKind() != lltok::APSInt)
     return TokError("expected signed integer");
 
   auto &S = Lex.getAPSIntVal();
   if (S < Result.Min)

 }
 
 /// ParseDISubprogram:
 ///   ::= !DISubprogram(scope: !0, name: "foo", linkageName: "_Zfoo",
 ///                     file: !1, line: 7, type: !2, isLocal: false,
 ///                     isDefinition: true, scopeLine: 8, containingType: !3,
 ///                     virtuality: DW_VIRTUALTIY_pure_virtual,
 ///                     virtualIndex: 10, thisAdjustment: 4, flags: 11,
-///                     isOptimized: false, templateParams: !4, declaration: !5,
-///                     retainedNodes: !6, thrownTypes: !7)
+///                     spFlags: 10, isOptimized: false, templateParams: !4,
+///                     declaration: !5, retainedNodes: !6, thrownTypes: !7)
 bool LLParser::ParseDISubprogram(MDNode *&Result, bool IsDistinct) {
   auto Loc = Lex.getLoc();
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   OPTIONAL(scope, MDField, );                                                  \
   OPTIONAL(name, MDStringField, );                                             \
   OPTIONAL(linkageName, MDStringField, );                                      \
   OPTIONAL(file, MDField, );                                                   \
   OPTIONAL(line, LineField, );                                                 \
   OPTIONAL(type, MDField, );                                                   \
   OPTIONAL(isLocal, MDBoolField, );                                            \
   OPTIONAL(isDefinition, MDBoolField, (true));                                 \
   OPTIONAL(scopeLine, LineField, );                                            \
   OPTIONAL(containingType, MDField, );                                         \
   OPTIONAL(virtuality, DwarfVirtualityField, );                                \
   OPTIONAL(virtualIndex, MDUnsignedField, (0, UINT32_MAX));                    \
   OPTIONAL(thisAdjustment, MDSignedField, (0, INT32_MIN, INT32_MAX));          \
   OPTIONAL(flags, DIFlagField, );                                              \
+  OPTIONAL(spFlags, DISPFlagField, );                                          \
   OPTIONAL(isOptimized, MDBoolField, );                                        \
   OPTIONAL(unit, MDField, );                                                   \
   OPTIONAL(templateParams, MDField, );                                         \
   OPTIONAL(declaration, MDField, );                                            \
-  OPTIONAL(retainedNodes, MDField, );                                              \
+  OPTIONAL(retainedNodes, MDField, );                                          \
   OPTIONAL(thrownTypes, MDField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
-  if (isDefinition.Val && !IsDistinct)
+  // An explicit spFlags field takes precedence over individual fields in
+  // older IR versions.
+  DISubprogram::DISPFlags SPFlags =
+      spFlags.Seen ? spFlags.Val
+                   : DISubprogram::toSPFlags(isLocal.Val, isDefinition.Val,
+                                             isOptimized.Val, virtuality.Val);
+  if ((SPFlags & DISubprogram::SPFlagDefinition) && !IsDistinct)
     return Lex.Error(
         Loc,
-        "missing 'distinct', required for !DISubprogram when 'isDefinition'");
-  DISubprogram::DISPFlags SPFlags = DISubprogram::toSPFlags(
-      isLocal.Val, isDefinition.Val, isOptimized.Val, virtuality.Val);
+        "missing 'distinct', required for !DISubprogram that is a Definition");
   Result = GET_OR_DISTINCT(
       DISubprogram,
       (Context, scope.Val, name.Val, linkageName.Val, file.Val, line.Val,
        type.Val, scopeLine.Val, containingType.Val, virtualIndex.Val,
        thisAdjustment.Val, flags.Val, SPFlags, unit.Val, templateParams.Val,
        declaration.Val, retainedNodes.Val, thrownTypes.Val));
   return false;
 }

llvm/lib/AsmParser/LLToken.h

enum Kind {
   DwarfAttEncoding, // DW_ATE_foo
   DwarfVirtuality,  // DW_VIRTUALITY_foo
   DwarfLang,        // DW_LANG_foo
   DwarfCC,          // DW_CC_foo
   EmissionKind,     // lineTablesOnly
   NameTableKind,    // GNU
   DwarfOp,          // DW_OP_foo
   DIFlag,           // DIFlagFoo
+  DISPFlag,         // DISPFlagFoo
   DwarfMacinfo,     // DW_MACINFO_foo
   ChecksumKind,     // CSK_foo
 
   // Type valued tokens (TyVal).
   Type,
 
   APFloat, // APFloatVal
   APSInt   // APSInt
 };
 } // end namespace lltok
 } // end namespace llvm
 
 #endif

llvm/lib/Bitcode/Reader/MetadataLoader.cpp

case bitc::METADATA_COMPILE_UNIT: {
     if (Metadata *SPs = getMDOrNullWithoutPlaceholders(Record[11]))
       CUSubprograms.push_back({CU, SPs});
     break;
   }
   case bitc::METADATA_SUBPROGRAM: {
     if (Record.size() < 18 || Record.size() > 21)
       return error("Invalid record");
 
-    IsDistinct =
-        (Record[0] & 1) || Record[8]; // All definitions should be distinct.
+    bool HasSPFlags = Record[0] & 4;
+    DISubprogram::DISPFlags SPFlags =
+        HasSPFlags
+            ? static_cast<DISubprogram::DISPFlags>(Record[11])
+            : DISubprogram::toSPFlags(
+                  /*IsLocalToUnit=*/Record[7], /*IsDefinition=*/Record[8],
+                  /*IsOptimized=*/Record[14], /*Virtuality=*/Record[11]);
+
+    // All definitions should be distinct.
+    IsDistinct = (Record[0] & 1) || (SPFlags & DISubprogram::SPFlagDefinition);
     // Version 1 has a Function as Record[15].
     // Version 2 has removed Record[15].
     // Version 3 has the Unit as Record[15].
     // Version 4 added thisAdjustment.
-    bool HasUnit = Record[0] >= 2;
+    bool HasUnit = Record[0] & 2;
     if (HasUnit && Record.size() < 19)
       return error("Invalid record");
     Metadata *CUorFn = getMDOrNull(Record[15]);
     unsigned Offset = Record.size() >= 19 ? 1 : 0;
     bool HasFn = Offset && !HasUnit;
     bool HasThisAdj = Record.size() >= 20;
     bool HasThrownTypes = Record.size() >= 21;
-    DISubprogram::DISPFlags SPFlags = DISubprogram::toSPFlags(
-        /*IsLocalToUnit=*/Record[7], /*IsDefinition=*/Record[8],
-        /*IsOptimized=*/Record[14], /*Virtuality=*/Record[11]);
     DISubprogram *SP = GET_OR_DISTINCT(
         DISubprogram,
         (Context,
          getDITypeRefOrNull(Record[1]),                     // scope
          getMDString(Record[2]),                            // name
          getMDString(Record[3]),                            // linkageName
          getMDOrNull(Record[4]),                            // file
          Record[5],                                         // line

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

void ModuleBitcodeWriter::writeDICompileUnit(const DICompileUnit *N,
 
   Stream.EmitRecord(bitc::METADATA_COMPILE_UNIT, Record, Abbrev);
   Record.clear();
 }
 
 void ModuleBitcodeWriter::writeDISubprogram(const DISubprogram *N,
                                             SmallVectorImpl<uint64_t> &Record,
                                             unsigned Abbrev) {
-  uint64_t HasUnitFlag = 1 << 1;
-  Record.push_back(N->isDistinct() | HasUnitFlag);
+  const uint64_t HasUnitFlag = 1 << 1;
+  const uint64_t HasSPFlagsFlag = 1 << 2;
+  Record.push_back(uint64_t(N->isDistinct()) | HasUnitFlag | HasSPFlagsFlag);
   Record.push_back(VE.getMetadataOrNullID(N->getScope()));
   Record.push_back(VE.getMetadataOrNullID(N->getRawName()));
   Record.push_back(VE.getMetadataOrNullID(N->getRawLinkageName()));
   Record.push_back(VE.getMetadataOrNullID(N->getFile()));
   Record.push_back(N->getLine());
   Record.push_back(VE.getMetadataOrNullID(N->getType()));
-  Record.push_back(N->isLocalToUnit());
-  Record.push_back(N->isDefinition());
+  Record.push_back(0); // unused
+  Record.push_back(0); // unused
   Record.push_back(N->getScopeLine());
   Record.push_back(VE.getMetadataOrNullID(N->getContainingType()));
-  Record.push_back(N->getVirtuality());
+  Record.push_back(N->getSPFlags());
   Record.push_back(N->getVirtualIndex());
   Record.push_back(N->getFlags());
-  Record.push_back(N->isOptimized());
+  Record.push_back(0); // unused
   Record.push_back(VE.getMetadataOrNullID(N->getRawUnit()));
   Record.push_back(VE.getMetadataOrNullID(N->getTemplateParams().get()));
   Record.push_back(VE.getMetadataOrNullID(N->getDeclaration()));
   Record.push_back(VE.getMetadataOrNullID(N->getRetainedNodes().get()));
   Record.push_back(N->getThisAdjustment());
   Record.push_back(VE.getMetadataOrNullID(N->getThrownTypes().get()));
 
   Stream.EmitRecord(bitc::METADATA_SUBPROGRAM, Record, Abbrev);

llvm/lib/IR/AsmWriter.cpp

struct MDFieldPrinter {
   void printString(StringRef Name, StringRef Value,
                    bool ShouldSkipEmpty = true);
   void printMetadata(StringRef Name, const Metadata *MD,
                      bool ShouldSkipNull = true);
   template <class IntTy>
   void printInt(StringRef Name, IntTy Int, bool ShouldSkipZero = true);
   void printBool(StringRef Name, bool Value, Optional<bool> Default = None);
   void printDIFlags(StringRef Name, DINode::DIFlags Flags);
+  void printDISPFlags(StringRef Name, DISubprogram::DISPFlags Flags);
   template <class IntTy, class Stringifier>
   void printDwarfEnum(StringRef Name, IntTy Value, Stringifier toString,
                       bool ShouldSkipZero = true);
   void printEmissionKind(StringRef Name, DICompileUnit::DebugEmissionKind EK);
   void printNameTableKind(StringRef Name,
                           DICompileUnit::DebugNameTableKind NTK);
 };
 
for (auto F : SplitFlags) {
     auto StringF = DINode::getFlagString(F);
     assert(!StringF.empty() && "Expected valid flag");
     Out << FlagsFS << StringF;
   }
   if (Extra || SplitFlags.empty())
     Out << FlagsFS << Extra;
 }
 
+void MDFieldPrinter::printDISPFlags(StringRef Name,
+                                    DISubprogram::DISPFlags Flags) {
+  // Always print this field, because no flags in the IR at all will be
+  // interpreted as old-style isDefinition: true.
+  Out << FS << Name << ": ";
+
+  if (!Flags) {
+    Out << 0;
+    return;
+  }
+
+  SmallVector<DISubprogram::DISPFlags, 8> SplitFlags;
+  auto Extra = DISubprogram::splitFlags(Flags, SplitFlags);
+
+  FieldSeparator FlagsFS(" | ");
+  for (auto F : SplitFlags) {
+    auto StringF = DISubprogram::getFlagString(F);
+    assert(!StringF.empty() && "Expected valid flag");
+    Out << FlagsFS << StringF;
+  }
+  if (Extra || SplitFlags.empty())
+    Out << FlagsFS << Extra;
+}
+
 void MDFieldPrinter::printEmissionKind(StringRef Name,
                                        DICompileUnit::DebugEmissionKind EK) {
   Out << FS << Name << ": " << DICompileUnit::emissionKindString(EK);
 }
 
 void MDFieldPrinter::printNameTableKind(StringRef Name,
                                         DICompileUnit::DebugNameTableKind NTK) {
   if (NTK == DICompileUnit::DebugNameTableKind::Default)
static void writeDISubprogram(raw_ostream &Out, const DISubprogram *N,
   Out << "!DISubprogram(";
   MDFieldPrinter Printer(Out, TypePrinter, Machine, Context);
   Printer.printString("name", N->getName());
   Printer.printString("linkageName", N->getLinkageName());
   Printer.printMetadata("scope", N->getRawScope(), /* ShouldSkipNull */ false);
   Printer.printMetadata("file", N->getRawFile());
   Printer.printInt("line", N->getLine());
   Printer.printMetadata("type", N->getRawType());
-  Printer.printBool("isLocal", N->isLocalToUnit());
-  Printer.printBool("isDefinition", N->isDefinition());
   Printer.printInt("scopeLine", N->getScopeLine());
   Printer.printMetadata("containingType", N->getRawContainingType());
-  Printer.printDwarfEnum("virtuality", N->getVirtuality(),
-                         dwarf::VirtualityString);
   if (N->getVirtuality() != dwarf::DW_VIRTUALITY_none ||
       N->getVirtualIndex() != 0)
     Printer.printInt("virtualIndex", N->getVirtualIndex(), false);
   Printer.printInt("thisAdjustment", N->getThisAdjustment());
   Printer.printDIFlags("flags", N->getFlags());
-  Printer.printBool("isOptimized", N->isOptimized());
+  Printer.printDISPFlags("spFlags", N->getSPFlags());
   Printer.printMetadata("unit", N->getRawUnit());
   Printer.printMetadata("templateParams", N->getRawTemplateParams());
   Printer.printMetadata("declaration", N->getRawDeclaration());
   Printer.printMetadata("retainedNodes", N->getRawRetainedNodes());
   Printer.printMetadata("thrownTypes", N->getRawThrownTypes());
   Out << ")";
 }
 

llvm/lib/IR/DebugInfoMetadata.cpp

 }
 
 DILocalScope *DILocalScope::getNonLexicalBlockFileScope() const {
   if (auto *File = dyn_cast<DILexicalBlockFile>(this))
     return File->getScope()->getNonLexicalBlockFileScope();
   return const_cast<DILocalScope *>(this);
 }
 
+DISubprogram::DISPFlags DISubprogram::getFlag(StringRef Flag) {
+  return StringSwitch<DISPFlags>(Flag)
+#define HANDLE_DISP_FLAG(ID, NAME) .Case("DISPFlag" #NAME, SPFlag##NAME)
+#include "llvm/IR/DebugInfoFlags.def"
+      .Default(SPFlagZero);
+}
+
+StringRef DISubprogram::getFlagString(DISPFlags Flag) {
+  switch (Flag) {
+  case SPFlagVirtuality: // Appease a warning.
+    return "";
+#define HANDLE_DISP_FLAG(ID, NAME)                                             \
+  case SPFlag##NAME:                                                           \
+    return "DISPFlag" #NAME;
+#include "llvm/IR/DebugInfoFlags.def"
+  }
+  return "";
+}
+
+DISubprogram::DISPFlags
+DISubprogram::splitFlags(DISPFlags Flags,
+                         SmallVectorImpl<DISPFlags> &SplitFlags) {
+  // Multi-bit fields can require special handling. In our case, however, the
+  // only multi-bit field is virtuality, and all its values happen to be
+  // single-bit values, so the right behavior just falls out.
+#define HANDLE_DISP_FLAG(ID, NAME)                                             \
+  if (DISPFlags Bit = Flags & SPFlag##NAME) {                                  \
+    SplitFlags.push_back(Bit);                                                 \
+    Flags &= ~Bit;                                                             \
+  }
+#include "llvm/IR/DebugInfoFlags.def"
+  return Flags;
+}
+
 DISubprogram *DISubprogram::getImpl(
     LLVMContext &Context, Metadata *Scope, MDString *Name,
     MDString *LinkageName, Metadata *File, unsigned Line, Metadata *Type,
     unsigned ScopeLine, Metadata *ContainingType, unsigned VirtualIndex,
     int ThisAdjustment, DIFlags Flags, DISPFlags SPFlags, Metadata *Unit,
     Metadata *TemplateParams, Metadata *Declaration, Metadata *RetainedNodes,
     Metadata *ThrownTypes, StorageType Storage, bool ShouldCreate) {
   assert(isCanonical(Name) && "Expected canonical MDString");

llvm/test/Assembler/disubprogram.ll

 ; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s
 ; RUN: verify-uselistorder %s
 
 ; CHECK: define void @_Z3foov() !dbg !9
 define void @_Z3foov() !dbg !9 {
   ret void
 }
 
-; CHECK: !named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14}
-!named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14}
+; CHECK: !named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15}
+!named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15}
 
 !0 = !{null}
 !1 = distinct !DICompositeType(tag: DW_TAG_structure_type)
 !2 = !DIFile(filename: "path/to/file", directory: "/path/to/dir")
 !3 = !DISubroutineType(types: !0)
 !4 = distinct !DICompositeType(tag: DW_TAG_structure_type)
 !5 = distinct !{}
 !6 = distinct !{}
 
-; CHECK: !7 = distinct !DISubprogram(scope: null, isLocal: false, isDefinition: true, isOptimized: false, unit: !8)
+; CHECK: !7 = distinct !DISubprogram(scope: null, spFlags: DISPFlagDefinition, unit: !8)
 !7 = distinct !DISubprogram(unit: !8)
 
 !8 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "clang",
                              file: !2,
                              isOptimized: true, flags: "-O2",
                              splitDebugFilename: "abc.debug", emissionKind: 2)
 
-; CHECK: !9 = !DISubprogram(scope: null, isLocal: false, isDefinition: false, isOptimized: false)
+; CHECK: !9 = !DISubprogram(scope: null, spFlags: 0)
 !9 = !DISubprogram(isDefinition: false)
 
-; CHECK: !10 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1, file: !2, line: 7, type: !3, isLocal: true, isDefinition: true, scopeLine: 8, containingType: !4, virtuality: DW_VIRTUALITY_pure_virtual, virtualIndex: 10, thisAdjustment: 3, flags: DIFlagPrototyped | DIFlagNoReturn, isOptimized: true, unit: !8, templateParams: !5, declaration: !9, retainedNodes: !6)
+; CHECK: !10 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1, file: !2, line: 7, type: !3, scopeLine: 8, containingType: !4, virtualIndex: 10, thisAdjustment: 3, flags: DIFlagPrototyped | DIFlagNoReturn, spFlags: DISPFlagPureVirtual | DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !8, templateParams: !5, declaration: !9, retainedNodes: !6)
 !10 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1,
                             file: !2, line: 7, type: !3, isLocal: true,
                             isDefinition: true, scopeLine: 8,
                             containingType: !4,
                             virtuality: DW_VIRTUALITY_pure_virtual,
                             virtualIndex: 10, thisAdjustment: 3, flags: DIFlagPrototyped | DIFlagNoReturn,
                             isOptimized: true, unit: !8, templateParams: !5,
                             declaration: !9, retainedNodes: !6)
!12 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1,
                             containingType: !4,
                             virtuality: DW_VIRTUALITY_none,
                             flags: DIFlagPrototyped, isOptimized: true,
                             unit: !8,
                             templateParams: !5, declaration: !9, retainedNodes: !6)
 
 !13 = !{!4}
 ; CHECK: !13 = !{!4}
-; CHECK: !14 = distinct !DISubprogram(name: "foo", scope: !1, file: !2, line: 1, type: !3, isLocal: true, isDefinition: true, scopeLine: 2, isOptimized: false, unit: !8, thrownTypes: !13)
+; CHECK: !14 = distinct !DISubprogram(name: "foo", scope: !1, file: !2, line: 1, type: !3, scopeLine: 2, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !8, thrownTypes: !13)
 !14 = distinct !DISubprogram(name: "foo", scope: !1,
                             file: !2, line: 1, type: !3, isLocal: true,
                             isDefinition: true, scopeLine: 2, isOptimized: false,
                             unit: !8, thrownTypes: !13)
 
-!15 = !{i32 1, !"Debug Info Version", i32 3}
-!llvm.module.flags = !{!15}
+; CHECK: !15 = distinct !DISubprogram({{.*}}, flags: DIFlagPrototyped, spFlags: DISPFlagPureVirtual | DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized,
+!15 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1,
+                             file: !2, line: 7, type: !3, scopeLine: 8,
+               containingType: !4, virtualIndex: 0,
+               flags: DIFlagPrototyped,
+               spFlags: DISPFlagPureVirtual | DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized,
+               unit: !8, templateParams: !5, declaration: !9,
+               retainedNodes: !6)
+
+!16 = !{i32 1, !"Debug Info Version", i32 3}
+!llvm.module.flags = !{!16}
 !llvm.dbg.cu = !{!8}

llvm/test/Assembler/invalid-disubprogram-uniqued-definition.ll

 ; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
 
-; CHECK: <stdin>:[[@LINE+1]]:6: error: missing 'distinct', required for !DISubprogram when 'isDefinition'
+; CHECK: <stdin>:[[@LINE+1]]:6: error: missing 'distinct', required for !DISubprogram that is a Definition
 !0 = !DISubprogram(isDefinition: true)

llvm/test/Bindings/llvm-c/debug_info.ll

 ; CHECK-NEXT: !12 = !{!13, !15}
 ; CHECK-NEXT: !13 = !DIImportedEntity(tag: DW_TAG_imported_module, scope: !6, entity: !14, file: !1, line: 42)
 ; CHECK-NEXT: !14 = !DIModule(scope: null, name: "llvm-c-test-import", includePath: "/test/include/llvm-c-test-import.h")
 ; CHECK-NEXT: !15 = !DIImportedEntity(tag: DW_TAG_imported_module, scope: !6, entity: !13, file: !1, line: 42)
 ; CHECK-NEXT: !16 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !17, size: 192, dwarfAddressSpace: 0)
 ; CHECK-NEXT: !17 = !DICompositeType(tag: DW_TAG_structure_type, name: "MyStruct", scope: !18, file: !1, size: 192, elements: !19, runtimeLang: DW_LANG_C89, identifier: "MyStruct")
 ; CHECK-NEXT: !18 = !DINamespace(name: "NameSpace", scope: !6)
 ; CHECK-NEXT: !19 = !{!11, !11, !11}
-; CHECK-NEXT: !20 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 42, type: !21, isLocal: true, isDefinition: true, scopeLine: 42, isOptimized: false, unit: !0, retainedNodes: !26)
+; CHECK-NEXT: !20 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 42, type: !21, scopeLine: 42, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !0, retainedNodes: !26)
 ; CHECK-NEXT: !21 = !DISubroutineType(types: !22)
 ; CHECK-NEXT: !22 = !{!11, !11, !23}
 ; CHECK-NEXT: !23 = !DICompositeType(tag: DW_TAG_array_type, baseType: !11, size: 640, flags: DIFlagVector, elements: !24)
 ; CHECK-NEXT: !24 = !{!25}
 ; CHECK-NEXT: !25 = !DISubrange(count: 10)
 ; CHECK-NEXT: !26 = !{!27, !28, !29, !30}
 ; CHECK-NEXT: !27 = !DILocalVariable(name: "a", arg: 1, scope: !20, file: !1, line: 42, type: !11)
 ; CHECK-NEXT: !28 = !DILocalVariable(name: "b", arg: 2, scope: !20, file: !1, line: 42, type: !11)
 ; CHECK-NEXT: !29 = !DILocalVariable(name: "c", arg: 3, scope: !20, file: !1, line: 42, type: !23)
 ; CHECK-NEXT: !30 = !DILocalVariable(name: "d", scope: !31, file: !1, line: 43, type: !11)
 ; CHECK-NEXT: !31 = distinct !DILexicalBlock(scope: !20, file: !1, line: 42)
 ; CHECK-NEXT: !32 = !DILocation(line: 42, scope: !20)
 ; CHECK-NEXT: !33 = !DILocation(line: 43, scope: !20)

llvm/test/Bitcode/DISubprogram-distinct-definitions.ll

 ; RUN: llvm-dis < %s.bc | FileCheck %s
 ; Check that subprogram definitions are correctly upgraded to 'distinct'.
 ; Bitcode compiled with llvm-as version 3.7.
 
 define void @f() !dbg !3 { ret void }
 
 !llvm.module.flags = !{!4}
 !llvm.dbg.cu = !{!0}
 !0 = distinct !DICompileUnit(language: 12, file: !1, subprograms: !{!3})
 !1 = !DIFile(filename: "path/to/file", directory: "/path/to/dir")
 
-; CHECK: = distinct !DISubprogram({{.*}}, isDefinition: true
+; CHECK: = distinct !DISubprogram({{.*}} DISPFlagDefinition
 !3 = !DISubprogram(name: "foo", isDefinition: true)
 !4 = !{i32 2, !"Debug Info Version", i32 3}

llvm/test/DebugInfo/Generic/invalid.ll

 ; RUN: llvm-as -disable-output %s 2>&1 | FileCheck %s
 
 ; Make sure we emit this diagnostic only once (which means we don't visit the
 ; same DISubprogram twice.
 ; CHECK: subprogram definitions must have a compile unit
-; CHECK-NEXT: !3 = distinct !DISubprogram(name: "patatino", scope: null, isLocal: false, isDefinition: true, isOptimized: false)
+; CHECK-NEXT: !3 = distinct !DISubprogram(name: "patatino", scope: null, spFlags: DISPFlagDefinition)
 ; CHECK-NOT: subprogram definitions must have a compile unit
-; CHECK-NOT: !3 = distinct !DISubprogram(name: "patatino", scope: null, isLocal: false, isDefinition: true, isOptimized: false)
+; CHECK-NOT: !3 = distinct !DISubprogram(name: "patatino", scope: null, spFlags: DISPFlagDefinition)
 ; CHECK: warning: ignoring invalid debug info
 
 define void @tinkywinky() !dbg !3 { ret void }
 
 !llvm.module.flags = !{!4}
 !llvm.dbg.cu = !{!0}
 !0 = distinct !DICompileUnit(language: 12, file: !1)
 !1 = !DIFile(filename: "/home/davide", directory: "/home/davide")
 !3 = distinct !DISubprogram(name: "patatino", isDefinition: true)
 !4 = !{i32 2, !"Debug Info Version", i32 3}

llvm/test/DebugInfo/debugify.ll

 }
 
 ; CHECK-DAG: !llvm.dbg.cu = !{![[CU:.*]]}
 ; CHECK-DAG: !llvm.debugify = !{![[NUM_INSTS:.*]], ![[NUM_VARS:.*]]}
 ; CHECK-DAG: "Debug Info Version"
 
 ; CHECK-DAG: ![[CU]] = distinct !DICompileUnit(language: DW_LANG_C, file: {{.*}}, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: {{.*}})
 ; CHECK-DAG: !DIFile(filename: "<stdin>", directory: "/")
-; CHECK-DAG: distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: {{.*}}, line: 1, type: {{.*}}, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, unit: {{.*}}, retainedNodes: {{.*}})
-; CHECK-DAG: distinct !DISubprogram(name: "bar", linkageName: "bar", scope: null, file: {{.*}}, line: 2, type: {{.*}}, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized: true, unit: {{.*}}, retainedNodes: {{.*}})
+; CHECK-DAG: distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: {{.*}}, line: 1, type: {{.*}}, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: {{.*}}, retainedNodes: {{.*}})
+; CHECK-DAG: distinct !DISubprogram(name: "bar", linkageName: "bar", scope: null, file: {{.*}}, line: 2, type: {{.*}}, scopeLine: 2, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: {{.*}}, retainedNodes: {{.*}})
 
 ; --- DILocations
 ; CHECK-DAG: ![[RET1]] = !DILocation(line: 1, column: 1
 ; CHECK-DAG: ![[CALL1]] = !DILocation(line: 2, column: 1
 ; CHECK-DAG: ![[ADD1]] = !DILocation(line: 3, column: 1
 ; CHECK-DAG: ![[RET2]] = !DILocation(line: 4, column: 1
 ; CHECK-DAG: ![[musttail]] = !DILocation(line: 5, column: 1
 ; CHECK-DAG: ![[musttailRes]] = !DILocation(line: 6, column: 1

llvm/test/Linker/replaced-function-matches-first-subprogram.ll

 !5 = !DIFile(filename: "t1.cpp", directory: "/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d1")
 !6 = !DISubroutineType(types: !2)
 
 ; Extract out the file from the replaced subprogram.
 ; CHECK-DAG: ![[SP2:.*]] = distinct !DISubprogram({{.*}} file: ![[FILE:[0-9]+]],{{.*}}, unit: ![[CU2]]
 
 ; We can't use CHECK-NOT/CHECK-SAME with a CHECK-DAG, so rely on field order to
 ; prove that there's no function: here.
-; CHECK-DAG: ![[SP2r:.*]] = {{.*}}!DISubprogram({{.*}} isOptimized: false, unit: ![[CU1]], retainedNodes:
+; CHECK-DAG: ![[SP2r:.*]] = {{.*}}!DISubprogram({{.*}} unit: ![[CU1]], retainedNodes:
 !7 = distinct !DISubprogram(name: "foo", line: 2, isLocal: false, isDefinition: true, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 2, file: !8, scope: !9, type: !6, retainedNodes: !2)
 
 ; The new subprogram should be pointing at the new directory.
 ; CHECK-DAG: ![[FILE]] = !DIFile(filename: "../t.h", directory: "/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d2")
 !8 = !DIFile(filename: "../t.h", directory: "/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d1")
 !9 = !DIFile(filename: "../t.h", directory: "/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d1")
 !10 = !{i32 2, !"Dwarf Version", i32 2}
 !11 = !{i32 2, !"Debug Info Version", i32 3}
 !12 = !{i32 1, !"PIC Level", i32 2}
 !13 = !{!"clang version 3.6.0 (trunk 224193) (llvm/trunk 224197)"}
 !14 = !DILocation(line: 2, column: 20, scope: !4)
 
 ; The same subprogram should be pointed to by inside the !dbg reference.
 ; CHECK: ![[LOC]] = !DILocation(line: 2, column: 15, scope: ![[SP2]])
 !15 = !DILocation(line: 2, column: 15, scope: !7)