[DebugInfo] Unify ChecksumKind and Checksum value in DIFile

[scott.linder]

Summary

Rather than encode the absence of a checksum with a Kind variant, instead put both the kind and value in a struct and wrap it in an Optional.

[aprantl]

include/llvm/IR/DebugInfoMetadata.h

502 ↗

(On Diff #133295)

These should be /// Doxygen comments.

519 ↗

(On Diff #133295)

If this is a string, why does this need to be a template?

lib/Bitcode/Reader/MetadataLoader.cpp

1357 ↗

(On Diff #133295)

When changing Bitcode, please always add a .bc upgrade test of the old format to test/Bitcode

[JDevlieghere]

include/llvm/IR/DebugInfoMetadata.h

504 ↗

(On Diff #133295)

Why did you not choose to make this part of the ChecksumInfo struct?

519 ↗

(On Diff #133295)

It's used for both MDStrings and StringRefs.

585 ↗

(On Diff #133295)

Would it make sense to move these into ChecksumInfo too?

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

177 ↗

(On Diff #133295)

Why do we have/need two enums to express the same thing? Are the values different?

[scott.linder]

Added Doxygen comment for DIFile::ChecksumKind.

[scott.linder]

include/llvm/IR/DebugInfoMetadata.h

504 ↗

(On Diff #133295)

They do not depend on the template parameter, so this saves having two instances of ChecksumKind in the typesystem, namely ChecksumInfo<MDString *>::ChecksumKind and ChecksumInfo<StringRef>::ChecksumKind.

585 ↗

(On Diff #133295)

Similar to above, these are static methods which do not depend on the templated string type (and never will), so I chose to leave them outside. Moving them inside just creates multiple, equivalent names for them.

[scott.linder]

Add Bitcode test generated with LLVM 5.0.1 to check backwards compatibility.

[scott.linder]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

177 ↗

(On Diff #133295)

The CodeView enum was already defined, but the previous code simply cast the DIFile::ChecksumKind to unsigned here, as the values happen to line up. This is somewhat fragile, and will only work for one format (why align the DIFile encoding with the CodeView encoding? why not DWARF?). I think being explicit is better here, and so when I had to make a change anyway I added the switch.

[JDevlieghere]

Thanks Scott! This LGTM, but let's wait for @aprantl to have another look before merging this.

include/llvm/IR/DebugInfoMetadata.h

504 ↗

(On Diff #133295)

Makes sense, thanks!

[aprantl]

lib/Bitcode/Writer/BitcodeWriter.cpp

1555 ↗

(On Diff #133437)

The comment in MetadataLoader claims that the None kind uses 0 as a representation. Why is this condition necessary?

test/Bitcode/upgrade-dbg-checksum.ll

16 ↗

(On Diff #133437)

I don't think you need a function or a global at all. An empty module with just two DICompileUnits should be sufficient.

[scott.linder]

lib/Bitcode/Writer/BitcodeWriter.cpp

1555 ↗

(On Diff #133437)

I'm not sure I understand what you are asking; the old code would have implicitly done the same here, as CSK_None was encoded as 0. In the new code the None variant of the Optional does not have an "encoding", so we have to explicitly encode each part of the struct, even if the Optional is None. In order to retain backwards compatibility, we still reserve null bytes for this purpose, and I documented this in comments on each end and in the ChecksumKind enum.

test/Bitcode/upgrade-dbg-checksum.ll

16 ↗

(On Diff #133437)

You are right, I will minimize the test case.

[scott.linder]

Minimized the Bitcode test case.

llvm/trunk/include/llvm/IR/DIBuilder.h

createCompileUnit(unsigned Lang, DIFile *File, StringRef Producer,
                           DICompileUnit::DebugEmissionKind::FullDebug,
                       uint64_t DWOId = 0, bool SplitDebugInlining = true,
                       bool DebugInfoForProfiling = false,
                       bool GnuPubnames = false);
 
     /// Create a file descriptor to hold debugging information for a file.
     /// \param Filename  File name.
     /// \param Directory Directory.
-    /// \param CSKind    Checksum kind (e.g. CSK_None, CSK_MD5, CSK_SHA1, etc.).
-    /// \param Checksum  Checksum data.
-    DIFile *createFile(StringRef Filename, StringRef Directory,
-                       DIFile::ChecksumKind CSKind = DIFile::CSK_None,
-                       StringRef Checksum = StringRef());
+    /// \param Checksum  Checksum kind (e.g. CSK_MD5, CSK_SHA1, etc.) and value.
+    DIFile *
+    createFile(StringRef Filename, StringRef Directory,
+               Optional<DIFile::ChecksumInfo<StringRef>> Checksum = None);
 
     /// Create debugging information entry for a macro.
     /// \param Parent     Macro parent (could be nullptr).
     /// \param Line       Source line number where the macro is defined.
     /// \param MacroType  DW_MACINFO_define or DW_MACINFO_undef.
     /// \param Name       Macro name.
     /// \param Value      Macro value.
     DIMacro *createMacro(DIMacroFile *Parent, unsigned Line, unsigned MacroType,

llvm/trunk/include/llvm/IR/DebugInfoMetadata.h

 ///
 /// TODO: Merge with directory/file node (including users).
 /// TODO: Canonicalize paths on creation.
 class DIFile : public DIScope {
   friend class LLVMContextImpl;
   friend class MDNode;
 
 public:
-  // These values must be explictly set, as they end up in the final object
-  // file.
+  /// Which algorithm (e.g. MD5) a checksum was generated with.
+  ///
+  /// The encoding is explicit because it is used directly in Bitcode. The
+  /// value 0 is reserved to indicate the absence of a checksum in Bitcode.
   enum ChecksumKind {
-    CSK_None = 0,
+    // The first variant was originally CSK_None, encoded as 0. The new
+    // internal representation removes the need for this by wrapping the
+    // ChecksumInfo in an Optional, but to preserve Bitcode compatibility the 0
+    // encoding is reserved.
     CSK_MD5 = 1,
     CSK_SHA1 = 2,
     CSK_Last = CSK_SHA1 // Should be last enumeration.
   };
 
+  /// A single checksum, represented by a \a Kind and a \a Value (a string).
+  template <typename T>
+  struct ChecksumInfo {
+    /// The kind of checksum which \a Value encodes.
+    ChecksumKind Kind;
+    /// The string value of the checksum.
+    T Value;
+
+    ChecksumInfo(ChecksumKind Kind, T Value) : Kind(Kind), Value(Value) { }
+    ~ChecksumInfo() = default;
+    bool operator==(const ChecksumInfo<T> &X) const {
+      return Kind == X.Kind && Value == X.Value;
+    }
+    bool operator!=(const ChecksumInfo<T> &X) const { return !(*this == X); }
+    StringRef getKindAsString() const { return getChecksumKindAsString(Kind); }
+  };
+
 private:
-  ChecksumKind CSKind;
+  Optional<ChecksumInfo<MDString *>> Checksum;
 
-  DIFile(LLVMContext &C, StorageType Storage, ChecksumKind CSK,
+  DIFile(LLVMContext &C, StorageType Storage,
+         Optional<ChecksumInfo<MDString *>> CS,
          ArrayRef<Metadata *> Ops)
       : DIScope(C, DIFileKind, Storage, dwarf::DW_TAG_file_type, Ops),
-        CSKind(CSK) {}
+        Checksum(CS) {}
   ~DIFile() = default;
 
   static DIFile *getImpl(LLVMContext &Context, StringRef Filename,
-                         StringRef Directory, ChecksumKind CSK, StringRef CS,
+                         StringRef Directory,
+                         Optional<ChecksumInfo<StringRef>> CS,
                          StorageType Storage, bool ShouldCreate = true) {
+    Optional<ChecksumInfo<MDString *>> MDChecksum;
+    if (CS)
+      MDChecksum.emplace(CS->Kind, getCanonicalMDString(Context, CS->Value));
     return getImpl(Context, getCanonicalMDString(Context, Filename),
-                   getCanonicalMDString(Context, Directory), CSK,
-                   getCanonicalMDString(Context, CS), Storage, ShouldCreate);
+                   getCanonicalMDString(Context, Directory), MDChecksum,
+                   Storage, ShouldCreate);
   }
   static DIFile *getImpl(LLVMContext &Context, MDString *Filename,
-                         MDString *Directory, ChecksumKind CSK, MDString *CS,
+                         MDString *Directory,
+                         Optional<ChecksumInfo<MDString *>> CS,
                          StorageType Storage, bool ShouldCreate = true);
 
   TempDIFile cloneImpl() const {
     return getTemporary(getContext(), getFilename(), getDirectory(),
-                        getChecksumKind(), getChecksum());
+                        getChecksum());
   }
 
 public:
   DEFINE_MDNODE_GET(DIFile, (StringRef Filename, StringRef Directory,
-                             ChecksumKind CSK = CSK_None,
-                             StringRef CS = StringRef()),
-                    (Filename, Directory, CSK, CS))
+                             Optional<ChecksumInfo<StringRef>> CS = None),
+                    (Filename, Directory, CS))
   DEFINE_MDNODE_GET(DIFile, (MDString * Filename, MDString *Directory,
-                             ChecksumKind CSK = CSK_None,
-                             MDString *CS = nullptr),
-                    (Filename, Directory, CSK, CS))
+                             Optional<ChecksumInfo<MDString *>> CS = None),
+                    (Filename, Directory, CS))
 
   TempDIFile clone() const { return cloneImpl(); }
 
   StringRef getFilename() const { return getStringOperand(0); }
   StringRef getDirectory() const { return getStringOperand(1); }
-  StringRef getChecksum() const { return getStringOperand(2); }
-  ChecksumKind getChecksumKind() const { return CSKind; }
-  StringRef getChecksumKindAsString() const;
+  Optional<ChecksumInfo<StringRef>> getChecksum() const {
+    Optional<ChecksumInfo<StringRef>> StringRefChecksum;
+    if (Checksum)
+      StringRefChecksum.emplace(Checksum->Kind, Checksum->Value->getString());
+    return StringRefChecksum;
+  }
 
   MDString *getRawFilename() const { return getOperandAs<MDString>(0); }
   MDString *getRawDirectory() const { return getOperandAs<MDString>(1); }
-  MDString *getRawChecksum() const { return getOperandAs<MDString>(2); }
+  Optional<ChecksumInfo<MDString *>> getRawChecksum() const { return Checksum; }
 
-  static ChecksumKind getChecksumKind(StringRef CSKindStr);
+  static StringRef getChecksumKindAsString(ChecksumKind CSKind);
+  static Optional<ChecksumKind> getChecksumKind(StringRef CSKindStr);
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DIFileKind;
   }
 };
 
 StringRef DIScope::getFilename() const {
   if (auto *F = getFile())

llvm/trunk/lib/AsmParser/LLParser.cpp

MDStringField(bool AllowEmpty = true)
       : ImplTy(nullptr), AllowEmpty(AllowEmpty) {}
 };
 
 struct MDFieldList : public MDFieldImpl<SmallVector<Metadata *, 4>> {
   MDFieldList() : ImplTy(SmallVector<Metadata *, 4>()) {}
 };
 
 struct ChecksumKindField : public MDFieldImpl<DIFile::ChecksumKind> {
-  ChecksumKindField() : ImplTy(DIFile::CSK_None) {}
   ChecksumKindField(DIFile::ChecksumKind CSKind) : ImplTy(CSKind) {}
 };
 
 struct MDSignedOrMDField : MDEitherFieldImpl<MDSignedField, MDField> {
   MDSignedOrMDField(int64_t Default = 0, bool AllowNull = true)
       : ImplTy(MDSignedField(Default), MDField(AllowNull)) {}
 
   MDSignedOrMDField(int64_t Default, int64_t Min, int64_t Max,
bool LLParser::ParseMDField(LocTy Loc, StringRef Name, MDFieldList &Result) {
 
   Result.assign(std::move(MDs));
   return false;
 }
 
 template <>
 bool LLParser::ParseMDField(LocTy Loc, StringRef Name,
                             ChecksumKindField &Result) {
-  if (Lex.getKind() != lltok::ChecksumKind)
+  Optional<DIFile::ChecksumKind> CSKind =
+      DIFile::getChecksumKind(Lex.getStrVal());
+
+  if (Lex.getKind() != lltok::ChecksumKind || !CSKind)
     return TokError(
         "invalid checksum kind" + Twine(" '") + Lex.getStrVal() + "'");
 
-  DIFile::ChecksumKind CSKind = DIFile::getChecksumKind(Lex.getStrVal());
-
-  Result.assign(CSKind);
+  Result.assign(*CSKind);
   Lex.Lex();
   return false;
 }
 
 } // end namespace llvm
 
 template <class ParserTy>
 bool LLParser::ParseMDFieldsImplBody(ParserTy parseField) {
#undef VISIT_MD_FIELDS
   return false;
 }
 
 /// ParseDIFileType:
 ///   ::= !DIFileType(filename: "path/to/file", directory: "/path/to/dir"
 ///                   checksumkind: CSK_MD5,
 ///                   checksum: "000102030405060708090a0b0c0d0e0f")
 bool LLParser::ParseDIFile(MDNode *&Result, bool IsDistinct) {
+  // The default constructed value for checksumkind is required, but will never
+  // be used, as the parser checks if the field was actually Seen before using
+  // the Val.
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   REQUIRED(filename, MDStringField, );                                         \
   REQUIRED(directory, MDStringField, );                                        \
-  OPTIONAL(checksumkind, ChecksumKindField, );                                 \
+  OPTIONAL(checksumkind, ChecksumKindField, (DIFile::CSK_MD5));                \
   OPTIONAL(checksum, MDStringField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
+  Optional<DIFile::ChecksumInfo<MDString *>> OptChecksum;
+  if (checksumkind.Seen && checksum.Seen)
+    OptChecksum.emplace(checksumkind.Val, checksum.Val);
+  else if (checksumkind.Seen || checksum.Seen)
+    return Lex.Error("'checksumkind' and 'checksum' must be provided together");
+
   Result = GET_OR_DISTINCT(DIFile, (Context, filename.Val, directory.Val,
-                                    checksumkind.Val, checksum.Val));
+                                    OptChecksum));
   return false;
 }
 
 /// ParseDICompileUnit:
 ///   ::= !DICompileUnit(language: DW_LANG_C99, file: !0, producer: "clang",
 ///                      isOptimized: true, flags: "-O2", runtimeVersion: 1,
 ///                      splitDebugFilename: "abc.debug",
 ///                      emissionKind: FullDebug, enums: !1, retainedTypes: !2,

llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp

case bitc::METADATA_MODULE: {
     break;
   }
 
   case bitc::METADATA_FILE: {
     if (Record.size() != 3 && Record.size() != 5)
       return error("Invalid record");
 
     IsDistinct = Record[0];
+    Optional<DIFile::ChecksumInfo<MDString *>> Checksum;
+    // The BitcodeWriter writes null bytes into Record[3:4] when the Checksum
+    // is not present. This matches up with the old internal representation,
+    // and the old encoding for CSK_None in the ChecksumKind. The new
+    // representation reserves the value 0 in the ChecksumKind to continue to
+    // encode None in a backwards-compatible way.
+    if (Record.size() == 5 && Record[3] && Record[4])
+      Checksum.emplace(static_cast<DIFile::ChecksumKind>(Record[3]),
+                       getMDString(Record[4]));
     MetadataList.assignValue(
         GET_OR_DISTINCT(
             DIFile,
             (Context, getMDString(Record[1]), getMDString(Record[2]),
-             Record.size() == 3 ? DIFile::CSK_None
-                                : static_cast<DIFile::ChecksumKind>(Record[3]),
-             Record.size() == 3 ? nullptr : getMDString(Record[4]))),
+             Checksum)),
         NextMetadataNo);
     NextMetadataNo++;
     break;
   }
   case bitc::METADATA_COMPILE_UNIT: {
     if (Record.size() < 14 || Record.size() > 19)
       return error("Invalid record");
 

llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp

 }
 
 void ModuleBitcodeWriter::writeDIFile(const DIFile *N,
                                       SmallVectorImpl<uint64_t> &Record,
                                       unsigned Abbrev) {
   Record.push_back(N->isDistinct());
   Record.push_back(VE.getMetadataOrNullID(N->getRawFilename()));
   Record.push_back(VE.getMetadataOrNullID(N->getRawDirectory()));
-  Record.push_back(N->getChecksumKind());
-  Record.push_back(VE.getMetadataOrNullID(N->getRawChecksum()));
+  if (N->getRawChecksum()) {
+    Record.push_back(N->getRawChecksum()->Kind);
+    Record.push_back(VE.getMetadataOrNullID(N->getRawChecksum()->Value));
+  } else {
+    // Maintain backwards compatibility with the old internal representation of
+    // CSK_None in ChecksumKind by writing nulls here when Checksum is None.
+    Record.push_back(0);
+    Record.push_back(VE.getMetadataOrNullID(nullptr));
+  }
 
   Stream.EmitRecord(bitc::METADATA_FILE, Record, Abbrev);
   Record.clear();
 }
 
 void ModuleBitcodeWriter::writeDICompileUnit(const DICompileUnit *N,
                                              SmallVectorImpl<uint64_t> &Record,
                                              unsigned Abbrev) {

llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

 }
 
 unsigned CodeViewDebug::maybeRecordFile(const DIFile *F) {
   StringRef FullPath = getFullFilepath(F);
   unsigned NextId = FileIdMap.size() + 1;
   auto Insertion = FileIdMap.insert(std::make_pair(FullPath, NextId));
   if (Insertion.second) {
     // We have to compute the full filepath and emit a .cv_file directive.
-    std::string Checksum = fromHex(F->getChecksum());
+    ArrayRef<uint8_t> ChecksumAsBytes;
+    FileChecksumKind CSKind = FileChecksumKind::None;
+    if (F->getChecksum()) {
+      std::string Checksum = fromHex(F->getChecksum()->Value);
       void *CKMem = OS.getContext().allocate(Checksum.size(), 1);
       memcpy(CKMem, Checksum.data(), Checksum.size());
-    ArrayRef<uint8_t> ChecksumAsBytes(reinterpret_cast<const uint8_t *>(CKMem),
-                                      Checksum.size());
-    DIFile::ChecksumKind ChecksumKind = F->getChecksumKind();
+      ChecksumAsBytes = ArrayRef<uint8_t>(
+          reinterpret_cast<const uint8_t *>(CKMem), Checksum.size());
+      switch (F->getChecksum()->Kind) {
+      case DIFile::CSK_MD5:  CSKind = FileChecksumKind::MD5; break;
+      case DIFile::CSK_SHA1: CSKind = FileChecksumKind::SHA1; break;
+      }
+    }
     bool Success = OS.EmitCVFileDirective(NextId, FullPath, ChecksumAsBytes,
-                                          static_cast<unsigned>(ChecksumKind));
+                                          static_cast<unsigned>(CSKind));
     (void)Success;
     assert(Success && ".cv_file directive failed");
   }
   return Insertion.first->second;
 }
 
 CodeViewDebug::InlineSite &
 CodeViewDebug::getInlineSite(const DILocation *InlinedAt,

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

void DwarfUnit::addSectionOffset(DIE &Die, dwarf::Attribute Attribute,
   if (DD->getDwarfVersion() >= 4)
     addUInt(Die, Attribute, dwarf::DW_FORM_sec_offset, Integer);
   else
     addUInt(Die, Attribute, dwarf::DW_FORM_data4, Integer);
 }
 
 MD5::MD5Result *DwarfUnit::getMD5AsBytes(const DIFile *File) {
   assert(File);
-  if (File->getChecksumKind() != DIFile::CSK_MD5)
+  Optional<DIFile::ChecksumInfo<StringRef>> Checksum = File->getChecksum();
+  if (!Checksum || Checksum->Kind != DIFile::CSK_MD5)
     return nullptr;
 
   // Convert the string checksum to an MD5Result for the streamer.
   // The verifier validates the checksum so we assume it's okay.
   // An MD5 checksum is 16 bytes.
-  std::string Checksum = fromHex(File->getChecksum());
+  std::string ChecksumString = fromHex(Checksum->Value);
   void *CKMem = Asm->OutStreamer->getContext().allocate(16, 1);
-  memcpy(CKMem, Checksum.data(), 16);
+  memcpy(CKMem, ChecksumString.data(), 16);
   return reinterpret_cast<MD5::MD5Result *>(CKMem);
 }
 
 unsigned DwarfTypeUnit::getOrCreateSourceID(const DIFile *File) {
   return SplitLineTable
              ? SplitLineTable->getFile(File->getDirectory(),
                                        File->getFilename(), getMD5AsBytes(File))
              : getCU().getOrCreateSourceID(File);

llvm/trunk/lib/IR/AsmWriter.cpp

struct MDFieldPrinter {
   explicit MDFieldPrinter(raw_ostream &Out) : Out(Out) {}
   MDFieldPrinter(raw_ostream &Out, TypePrinting *TypePrinter,
                  SlotTracker *Machine, const Module *Context)
       : Out(Out), TypePrinter(TypePrinter), Machine(Machine), Context(Context) {
   }
 
   void printTag(const DINode *N);
   void printMacinfoType(const DIMacroNode *N);
-  void printChecksumKind(const DIFile *N);
+  void printChecksum(const DIFile::ChecksumInfo<StringRef> &N);
   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 MDFieldPrinter::printMacinfoType(const DIMacroNode *N) {
   Out << FS << "type: ";
   auto Type = dwarf::MacinfoString(N->getMacinfoType());
   if (!Type.empty())
     Out << Type;
   else
     Out << N->getMacinfoType();
 }
 
-void MDFieldPrinter::printChecksumKind(const DIFile *N) {
-  if (N->getChecksumKind() == DIFile::CSK_None)
-    // Skip CSK_None checksum kind.
-    return;
-  Out << FS << "checksumkind: " << N->getChecksumKindAsString();
+void MDFieldPrinter::printChecksum(
+    const DIFile::ChecksumInfo<StringRef> &Checksum) {
+  Out << FS << "checksumkind: " << Checksum.getKindAsString();
+  printString("checksum", Checksum.Value, /* ShouldSkipEmpty */ false);
 }
 
 void MDFieldPrinter::printString(StringRef Name, StringRef Value,
                                  bool ShouldSkipEmpty) {
   if (ShouldSkipEmpty && Value.empty())
     return;
 
   Out << FS << Name << ": \"";

 static void writeDIFile(raw_ostream &Out, const DIFile *N, TypePrinting *,
                         SlotTracker *, const Module *) {
   Out << "!DIFile(";
   MDFieldPrinter Printer(Out);
   Printer.printString("filename", N->getFilename(),
                       /* ShouldSkipEmpty */ false);
   Printer.printString("directory", N->getDirectory(),
                       /* ShouldSkipEmpty */ false);
-  Printer.printChecksumKind(N);
-  Printer.printString("checksum", N->getChecksum(), /* ShouldSkipEmpty */ true);
+  // Print all values for checksum together, or not at all.
+  if (N->getChecksum())
+    Printer.printChecksum(*N->getChecksum());
   Out << ")";
 }
 
 static void writeDICompileUnit(raw_ostream &Out, const DICompileUnit *N,
                                TypePrinting *TypePrinter, SlotTracker *Machine,
                                const Module *Context) {
   Out << "!DICompileUnit(";
   MDFieldPrinter Printer(Out, TypePrinter, Machine, Context);

llvm/trunk/lib/IR/DIBuilder.cpp

DIImportedEntity *DIBuilder::createImportedDeclaration(DIScope *Context,
   // Make sure to use the unique identifier based metadata reference for
   // types that have one.
   return ::createImportedModule(VMContext, dwarf::DW_TAG_imported_declaration,
                                 Context, Decl, File, Line, Name,
                                 AllImportedModules);
 }
 
 DIFile *DIBuilder::createFile(StringRef Filename, StringRef Directory,
-                              DIFile::ChecksumKind CSKind, StringRef Checksum) {
-  return DIFile::get(VMContext, Filename, Directory, CSKind, Checksum);
+                              Optional<DIFile::ChecksumInfo<StringRef>> CS) {
+  return DIFile::get(VMContext, Filename, Directory, CS);
 }
 
 DIMacro *DIBuilder::createMacro(DIMacroFile *Parent, unsigned LineNumber,
                                 unsigned MacroType, StringRef Name,
                                 StringRef Value) {
   assert(!Name.empty() && "Unable to create macro without name");
   assert((MacroType == dwarf::DW_MACINFO_undef ||
           MacroType == dwarf::DW_MACINFO_define) &&

llvm/trunk/lib/IR/DebugInfoMetadata.cpp

DISubroutineType *DISubroutineType::getImpl(LLVMContext &Context, DIFlags Flags,
                                             bool ShouldCreate) {
   DEFINE_GETIMPL_LOOKUP(DISubroutineType, (Flags, CC, TypeArray));
   Metadata *Ops[] = {nullptr, nullptr, nullptr, TypeArray};
   DEFINE_GETIMPL_STORE(DISubroutineType, (Flags, CC), Ops);
 }
 
 // FIXME: Implement this string-enum correspondence with a .def file and macros,
 // so that the association is explicit rather than implied.
-static const char *ChecksumKindName[DIFile::CSK_Last + 1] = {
-  "CSK_None",
+static const char *ChecksumKindName[DIFile::CSK_Last] = {
   "CSK_MD5",
   "CSK_SHA1"
 };
 
-DIFile::ChecksumKind DIFile::getChecksumKind(StringRef CSKindStr) {
-  return StringSwitch<DIFile::ChecksumKind>(CSKindStr)
-      .Case("CSK_MD5", DIFile::CSK_MD5)
-      .Case("CSK_SHA1", DIFile::CSK_SHA1)
-      .Default(DIFile::CSK_None);
+StringRef DIFile::getChecksumKindAsString(ChecksumKind CSKind) {
+  assert(CSKind <= DIFile::CSK_Last && "Invalid checksum kind");
+  // The first space was originally the CSK_None variant, which is now
+  // obsolete, but the space is still reserved in ChecksumKind, so we account
+  // for it here.
+  return ChecksumKindName[CSKind - 1];
 }
 
-StringRef DIFile::getChecksumKindAsString() const {
-  assert(CSKind <= DIFile::CSK_Last && "Invalid checksum kind");
-  return ChecksumKindName[CSKind];
+Optional<DIFile::ChecksumKind> DIFile::getChecksumKind(StringRef CSKindStr) {
+  return StringSwitch<Optional<DIFile::ChecksumKind>>(CSKindStr)
+      .Case("CSK_MD5", DIFile::CSK_MD5)
+      .Case("CSK_SHA1", DIFile::CSK_SHA1)
+      .Default(None);
 }
 
 DIFile *DIFile::getImpl(LLVMContext &Context, MDString *Filename,
-                        MDString *Directory, DIFile::ChecksumKind CSKind,
-                        MDString *Checksum, StorageType Storage,
-                        bool ShouldCreate) {
+                        MDString *Directory,
+                        Optional<DIFile::ChecksumInfo<MDString *>> CS,
+                        StorageType Storage, bool ShouldCreate) {
   assert(isCanonical(Filename) && "Expected canonical MDString");
   assert(isCanonical(Directory) && "Expected canonical MDString");
-  assert(isCanonical(Checksum) && "Expected canonical MDString");
-  DEFINE_GETIMPL_LOOKUP(DIFile, (Filename, Directory, CSKind, Checksum));
-  Metadata *Ops[] = {Filename, Directory, Checksum};
-  DEFINE_GETIMPL_STORE(DIFile, (CSKind), Ops);
+  assert((!CS || isCanonical(CS->Value)) && "Expected canonical MDString");
+  DEFINE_GETIMPL_LOOKUP(DIFile, (Filename, Directory, CS));
+  Metadata *Ops[] = {Filename, Directory, CS ? CS->Value : nullptr};
+  DEFINE_GETIMPL_STORE(DIFile, (CS), Ops);
 }
 
 DICompileUnit *DICompileUnit::getImpl(
     LLVMContext &Context, unsigned SourceLanguage, Metadata *File,
     MDString *Producer, bool IsOptimized, MDString *Flags,
     unsigned RuntimeVersion, MDString *SplitDebugFilename,
     unsigned EmissionKind, Metadata *EnumTypes, Metadata *RetainedTypes,
     Metadata *GlobalVariables, Metadata *ImportedEntities, Metadata *Macros,
DIMacroFile *DIMacroFile::getImpl(LLVMContext &Context, unsigned MIType,
                                   unsigned Line, Metadata *File,
                                   Metadata *Elements, StorageType Storage,
                                   bool ShouldCreate) {
   DEFINE_GETIMPL_LOOKUP(DIMacroFile,
                         (MIType, Line, File, Elements));
   Metadata *Ops[] = { File, Elements };
   DEFINE_GETIMPL_STORE(DIMacroFile, (MIType, Line), Ops);
 }
-

llvm/trunk/lib/IR/LLVMContextImpl.h

template <> struct MDNodeKeyImpl<DISubroutineType> {
   }
 
   unsigned getHashValue() const { return hash_combine(Flags, CC, TypeArray); }
 };
 
 template <> struct MDNodeKeyImpl<DIFile> {
   MDString *Filename;
   MDString *Directory;
-  DIFile::ChecksumKind CSKind;
-  MDString *Checksum;
+  Optional<DIFile::ChecksumInfo<MDString *>> Checksum;
 
   MDNodeKeyImpl(MDString *Filename, MDString *Directory,
-                DIFile::ChecksumKind CSKind, MDString *Checksum)
-      : Filename(Filename), Directory(Directory), CSKind(CSKind),
-        Checksum(Checksum) {}
+                Optional<DIFile::ChecksumInfo<MDString *>> Checksum)
+      : Filename(Filename), Directory(Directory), Checksum(Checksum) {}
   MDNodeKeyImpl(const DIFile *N)
       : Filename(N->getRawFilename()), Directory(N->getRawDirectory()),
-        CSKind(N->getChecksumKind()), Checksum(N->getRawChecksum()) {}
+        Checksum(N->getRawChecksum()) {}
 
   bool isKeyOf(const DIFile *RHS) const {
     return Filename == RHS->getRawFilename() &&
            Directory == RHS->getRawDirectory() &&
-           CSKind == RHS->getChecksumKind() &&
            Checksum == RHS->getRawChecksum();
   }
 
   unsigned getHashValue() const {
-    return hash_combine(Filename, Directory, CSKind, Checksum);
+    if (Checksum)
+      return hash_combine(Filename, Directory, Checksum->Kind, Checksum->Value);
+    return hash_combine(Filename, Directory);
   }
 };
 
 template <> struct MDNodeKeyImpl<DISubprogram> {
   Metadata *Scope;
   MDString *Name;
   MDString *LinkageName;
   Metadata *File;

llvm/trunk/lib/IR/Verifier.cpp

if (auto *Types = N.getRawTypeArray()) {
     }
   }
   AssertDI(!hasConflictingReferenceFlags(N.getFlags()),
            "invalid reference flags", &N);
 }
 
 void Verifier::visitDIFile(const DIFile &N) {
   AssertDI(N.getTag() == dwarf::DW_TAG_file_type, "invalid tag", &N);
-  AssertDI(N.getChecksumKind() <= DIFile::CSK_Last, "invalid checksum kind",
-           &N);
+  Optional<DIFile::ChecksumInfo<StringRef>> Checksum = N.getChecksum();
+  if (Checksum) {
+    AssertDI(Checksum->Kind <= DIFile::ChecksumKind::CSK_Last,
+             "invalid checksum kind", &N);
     size_t Size;
-  switch (N.getChecksumKind()) {
-  case DIFile::CSK_None:
-    Size = 0;
-    break;
+    switch (Checksum->Kind) {
     case DIFile::CSK_MD5:
       Size = 32;
       break;
     case DIFile::CSK_SHA1:
       Size = 40;
       break;
     }
-  AssertDI(N.getChecksum().size() == Size, "invalid checksum length", &N);
-  AssertDI(N.getChecksum().find_if_not(llvm::isHexDigit) == StringRef::npos,
+    AssertDI(Checksum->Value.size() == Size, "invalid checksum length", &N);
+    AssertDI(Checksum->Value.find_if_not(llvm::isHexDigit) == StringRef::npos,
              "invalid checksum", &N);
   }
+}
 
 void Verifier::visitDICompileUnit(const DICompileUnit &N) {
   AssertDI(N.isDistinct(), "compile units must be distinct", &N);
   AssertDI(N.getTag() == dwarf::DW_TAG_compile_unit, "invalid tag", &N);
 
   // Don't bother verifying the compilation directory or producer string
   // as those could be empty.
   AssertDI(N.getRawFile() && isa<DIFile>(N.getRawFile()), "invalid file", &N,

llvm/trunk/test/Assembler/debug-info.ll

 ; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s
 ; RUN: verify-uselistorder %s
 
-; CHECK: !named = !{!0, !0, !1, !2, !3, !4, !5, !6, !7, !8, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20, !21, !22, !23, !24, !25, !26, !27, !27, !28, !29, !30, !31, !32, !33, !34, !35, !36}
-!named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20, !21, !22, !23, !24, !25, !26, !27, !28, !29, !30, !31, !32, !33, !34, !35, !36, !38, !39, !40}
+; CHECK: !named = !{!0, !0, !1, !2, !3, !4, !5, !6, !7, !8, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20, !21, !22, !23, !24, !25, !26, !27, !27, !28, !29, !30, !31, !32, !33, !34, !35}
+!named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20, !21, !22, !23, !24, !25, !26, !27, !28, !29, !30, !31, !32, !33, !34, !35, !36, !37, !38}
 
 ; CHECK:      !0 = !DISubrange(count: 3)
 ; CHECK-NEXT: !1 = !DISubrange(count: 3, lowerBound: 4)
 ; CHECK-NEXT: !2 = !DISubrange(count: 3, lowerBound: -5)
 !0 = !DISubrange(count: 3)
 !1 = !DISubrange(count: 3, lowerBound: 0)
 
 !2 = !DISubrange(count: 3, lowerBound: 4)

 ; CHECK-NEXT: !30 = !DIMacroFile(line: 9, file: !12, nodes: !29)
 ; CHECK-NEXT: !31 = !DIMacroFile(line: 11, file: !12)
 !31 = !DIMacro(type: DW_MACINFO_define, line: 9, name: "Name", value: "Value")
 !32 = distinct !{!31}
 !33 = !DIMacroFile(line: 9, file: !14, nodes: !32)
 !34 = !DIMacroFile(type: DW_MACINFO_start_file, line: 11, file: !14)
 
 ; CHECK-NEXT: !32 = !DIFile(filename: "file", directory: "dir", checksumkind: CSK_MD5, checksum: "000102030405060708090a0b0c0d0e0f")
-; CHECK-NEXT: !33 = !DIFile(filename: "file", directory: "dir")
 !35 = !DIFile(filename: "file", directory: "dir", checksumkind: CSK_MD5, checksum: "000102030405060708090a0b0c0d0e0f")
-!36 = !DIFile(filename: "file", directory: "dir", checksumkind: CSK_None)
-!37 = !DIFile(filename: "file", directory: "dir", checksumkind: CSK_None, checksum: "")
 
-; CHECK-NEXT: !34 = !DICompositeType(tag: DW_TAG_variant_part, name: "A", scope: !14, size: 64)
-; CHECK-NEXT: !35 = !DIDerivedType(tag: DW_TAG_member, scope: !34, baseType: !36, size: 64, align: 64, flags: DIFlagArtificial)
-; CHECK-NEXT: !36 = !DIBasicType(name: "u64", size: 64, encoding: DW_ATE_unsigned)
-!38 = !DICompositeType(tag: DW_TAG_variant_part, name: "A", scope: !16, size: 64, discriminator: !39)
-!39 = !DIDerivedType(tag: DW_TAG_member, scope: !38, baseType: !40, size: 64, align: 64, flags: DIFlagArtificial)
-!40 = !DIBasicType(name: "u64", size: 64, encoding: DW_ATE_unsigned)
+; CHECK-NEXT: !33 = !DICompositeType(tag: DW_TAG_variant_part, name: "A", scope: !14, size: 64)
+; CHECK-NEXT: !34 = !DIDerivedType(tag: DW_TAG_member, scope: !33, baseType: !35, size: 64, align: 64, flags: DIFlagArtificial)
+; CHECK-NEXT: !35 = !DIBasicType(name: "u64", size: 64, encoding: DW_ATE_unsigned)
+!36 = !DICompositeType(tag: DW_TAG_variant_part, name: "A", scope: !16, size: 64, discriminator: !37)
+!37 = !DIDerivedType(tag: DW_TAG_member, scope: !36, baseType: !38, size: 64, align: 64, flags: DIFlagArtificial)
+!38 = !DIBasicType(name: "u64", size: 64, encoding: DW_ATE_unsigned)

llvm/trunk/test/Bitcode/upgrade-dbg-checksum.ll

+; Test that DIFile(checksumkind, checksum) representation in Bitcode does
+; not change.
+;
+; RUN: llvm-dis < %s.bc | FileCheck %s
+; RUN: verify-uselistorder < %s.bc
+
+!llvm.dbg.cu = !{!1, !2, !3}
+!llvm.module.flags = !{!7}
+
+!1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !4, producer: "clang version 5.0.1 (tags/RELEASE_501/final)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !5, producer: "clang version 5.0.1 (tags/RELEASE_501/final)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
+!3 = distinct !DICompileUnit(language: DW_LANG_C99, file: !6, producer: "clang version 5.0.1 (tags/RELEASE_501/final)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
+; CHECK: !DIFile(filename: "a.c", directory: "/test")
+!4 = !DIFile(filename: "a.c", directory: "/test", checksumkind: CSK_None, checksum: "")
+; CHECK: !DIFile(filename: "b.h", directory: "/test", checksumkind: CSK_MD5, checksum: "595f44fec1e92a71d3e9e77456ba80d1")
+!5 = !DIFile(filename: "b.h", directory: "/test", checksumkind: CSK_MD5, checksum: "595f44fec1e92a71d3e9e77456ba80d1")
+; CHECK: !DIFile(filename: "c.h", directory: "/test", checksumkind: CSK_SHA1, checksum: "d5db29cd03a2ed055086cef9c31c252b4587d6d0")
+!6 = !DIFile(filename: "c.h", directory: "/test", checksumkind: CSK_SHA1, checksum: "d5db29cd03a2ed055086cef9c31c252b4587d6d0")
+!7 = !{i32 2, !"Debug Info Version", i32 3}

llvm/trunk/test/Verifier/DIFile.ll

 ; RUN: llvm-as -disable-output < %s 2>&1 | FileCheck %s
 
 ; The lengths for None and MD5 are wrong; SHA1 has a non-hex digit.
-; CHECK: invalid checksum length
 ; CHECK: invalid checksum{{$}}
 ; CHECK: invalid checksum length
 ; CHECK: warning: ignoring invalid debug info in <stdin>
 
 @t1 = global i32 1, align 4, !dbg !0
 @t2 = global i32 0, align 4, !dbg !6
 
 !llvm.dbg.cu = !{!2}
 !llvm.module.flags = !{!11, !12, !13}
 !llvm.ident = !{!14}
 
 !0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
 !1 = distinct !DIGlobalVariable(name: "t1", scope: !2, file: !10, line: 1, type: !9, isLocal: false, isDefinition: true)
 !2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 7.0.0 (trunk 322159)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
-!3 = !DIFile(filename: "t.c", directory: "/scratch", checksumkind: CSK_None, checksum: "00")
+!3 = !DIFile(filename: "t.c", directory: "/scratch")
 !4 = !{}
 !5 = !{!0, !6}
 !6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
 !7 = distinct !DIGlobalVariable(name: "t2", scope: !2, file: !8, line: 1, type: !9, isLocal: false, isDefinition: true)
 !8 = !DIFile(filename: "./t2.h", directory: "/scratch", checksumkind: CSK_MD5, checksum: "2222")
 !9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
 !10 = !DIFile(filename: "./t1.h", directory: "/scratch", checksumkind: CSK_SHA1, checksum: "123456789012345678901234567890123456789.")
 !11 = !{i32 2, !"Dwarf Version", i32 4}
 !12 = !{i32 2, !"Debug Info Version", i32 3}
 !13 = !{i32 1, !"wchar_size", i32 4}
 !14 = !{!"clang version 7.0.0 (trunk 322159)"}

llvm/trunk/unittests/IR/MetadataTest.cpp

TEST_F(DISubroutineTypeTest, get) {
   EXPECT_EQ("", N->getName());
 }
 
 typedef MetadataTest DIFileTest;
 
 TEST_F(DIFileTest, get) {
   StringRef Filename = "file";
   StringRef Directory = "dir";
-  DIFile::ChecksumKind CSKind = DIFile::CSK_MD5;
-  StringRef Checksum = "000102030405060708090a0b0c0d0e0f";
-  auto *N = DIFile::get(Context, Filename, Directory, CSKind, Checksum);
+  DIFile::ChecksumKind CSKind = DIFile::ChecksumKind::CSK_MD5;
+  StringRef ChecksumString = "000102030405060708090a0b0c0d0e0f";
+  DIFile::ChecksumInfo<StringRef> Checksum(CSKind, ChecksumString);
+  auto *N = DIFile::get(Context, Filename, Directory, Checksum);
 
   EXPECT_EQ(dwarf::DW_TAG_file_type, N->getTag());
   EXPECT_EQ(Filename, N->getFilename());
   EXPECT_EQ(Directory, N->getDirectory());
-  EXPECT_EQ(CSKind, N->getChecksumKind());
   EXPECT_EQ(Checksum, N->getChecksum());
-  EXPECT_EQ(N, DIFile::get(Context, Filename, Directory, CSKind, Checksum));
+  EXPECT_EQ(N, DIFile::get(Context, Filename, Directory, Checksum));
 
-  EXPECT_NE(N, DIFile::get(Context, "other", Directory, CSKind, Checksum));
-  EXPECT_NE(N, DIFile::get(Context, Filename, "other", CSKind, Checksum));
+  EXPECT_NE(N, DIFile::get(Context, "other", Directory, Checksum));
+  EXPECT_NE(N, DIFile::get(Context, Filename, "other", Checksum));
+  DIFile::ChecksumInfo<StringRef> OtherChecksum(DIFile::ChecksumKind::CSK_SHA1, ChecksumString);
   EXPECT_NE(
-      N, DIFile::get(Context, Filename, Directory, DIFile::CSK_SHA1, Checksum));
+      N, DIFile::get(Context, Filename, Directory, OtherChecksum));
   EXPECT_NE(N, DIFile::get(Context, Filename, Directory));
 
   TempDIFile Temp = N->clone();
   EXPECT_EQ(N, MDNode::replaceWithUniqued(std::move(Temp)));
 }
 
 TEST_F(DIFileTest, ScopeGetFile) {
   // Ensure that DIScope::getFile() returns itself.