DebugInfo: Added support for Checksum debug info feature (LLVM IR part)

[aaboud]

Summary

Resolve Bug 30978.

This patch contains changes in LLVM IR to support Checksum debug info feature.

[mehdi_amini]

I think we try to make as much as possible metadata to be "distinct", CC Duncan to advise.

docs/LangRef.rst

4018 ↗

(On Diff #80976)

What are the valid list of "type"? What type is the "type" field? Why the prefix ChecksumType_ and not just MD5?
Why not a simpler approach where the type is an MDString? (That already how you parse it in DIChecksum::getChecksumType anyway)

include/llvm/IR/DebugInfoMetadata.h

430 ↗

(On Diff #80976)

Why there? Where is this API used?

491 ↗

(On Diff #80976)

What is the reason to have the "None"?

1331 ↗

(On Diff #80976)

Intended?

[aaboud]

In D27642#619007, @mehdi_amini wrote:

I think we try to make as much as possible metadata to be "distinct", CC Duncan to advise.

Not sure about that, so far we have only DICompileUnit and DISubprogram defined as distinct.
If I am not mistaken, distinct will prevent us from sharing Metadata, and for checksum there is no harm of sharing.

docs/LangRef.rst

4018 ↗

(On Diff #80976)

Right now the valid list of types are:
ChecksumType_MD5
ChecksumType_SHA1

I will add this to the document.

Answering your other questions:

1. The current solution does not handle the type as string, but as an enumeration with a known token (see LLexer.cpp)
2. Omitting the prefix ChecksumType_ will not allow us to recognize the token.
3. This is similar to what is done with DIFlag...
4. Allowing the user to write the type as string "MD5" is easier to implement, however, checking invalid types will not be elegant as this solution.

Bottom line, I am ready to go with either solutions.
Let's hear what others think.

include/llvm/IR/DebugInfoMetadata.h

430 ↗

(On Diff #80976)

We need to getChecksum from DICompileUnit, however, this information is stored in the DIScope.

491 ↗

(On Diff #80976)

We need to pass the checksum data and type to the DICompileUnit together with the filename and directory. However, not always we have a checksum data to send, and that is why we need an indicator that there is no checksum available.

If you try to create a checksum with None type Checksum::get() will return a nullptr.
This is the best way I could find to deal with the issue.
Please, let me know if you have better way to do that.

1331 ↗

(On Diff #80976)

I should have removed this line.
I am not sure why we have the getFilename() and getDirectory() for DILocation, thought that I might need the getChecksum for same reason, however, I guess it is not needed.

If for any reason we end up need this function we will add it then.
For now I will remove it.

[dexonsmith]

+Adrian.

Note that I'm not worried about using 'distinct' here, since DICheckSum doesn't look like it can participate in cycles. (Otherwise, I haven't looked deeply at the patch.)

[rnk]

Any objections to folding this into DIFile? It would substantially reduce the size of this patch, IMO.

docs/LangRef.rst

4018 ↗

(On Diff #80976)

I think we could fold DIChecksum into DIFile. It would look like:

!DIFile(filename: ..., checksumkind: ChecksumType_MD5, checksum: !"deadbeef")

This adds a dead integer to DIFile when checksums aren't being used, but it's probably more efficient and less work than having a separate node.

include/llvm/IR/DebugInfoMetadata.h

430 ↗

(On Diff #80976)

I don't believe we really need it. We can probably do CU->getFile()->getChecksum().

lib/IR/DIBuilder.cpp

127–128 ↗

(On Diff #80976)

Instead of having createCompileUnit take Filename, Directory, ChecksumType, and Checksum, have it take a DIFIle.

[aaboud]

In D27642#619933, @rnk wrote:

Any objections to folding this into DIFile? It would substantially reduce the size of this patch, IMO.

I have no objection going this way, if everybody agree on that I will implement the change and upload a new patch.

docs/LangRef.rst

4018 ↗

(On Diff #80976)

Fine with me.

include/llvm/IR/DebugInfoMetadata.h

430 ↗

(On Diff #80976)

OK, can do.

lib/IR/DIBuilder.cpp

127–128 ↗

(On Diff #80976)

It would be much better solution for sure.
I will give it a try (in different patch), unless you want to do it yourself :) ?

[aprantl]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

I'm in favor of rolling it into DIFile. As Paul noted, DWARF 5 also allows for an MD5 checksum on each file entry in the line table section so I'm expecting this to become the normal case eventually.

What's the problem with recognizing MD5 as a context-sensitive token in the parser?

[aaboud]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

It is not only MD5, right? should the LLexer recognize all possible types of checksum, i.e. MD5, SHA1, etc.?
Or like it is implemented in this patch, it only needs to recognize the checksum type prefix (whatever it will be).

So, how do you prefer to implement this?

1. Prefix - (if yes, what should be the prefix?)
2. Recognize all types
3. Have a wild string

[mehdi_amini]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

I rather have a much simpler implementation, the validation can be done in the verifier easily and it would handle similarly in-memory validation, bitcode and textual IR (the first two are the most important by the way, textual IR is not what we should optimize for).

With the change that @rnk suggested that would make it quite a smaller patch I believe.

4018 ↗

(On Diff #80976)

Alternative representation:

!DIFile(filename: ..., checksum: !"MD5:deadbeef")

lib/Bitcode/Reader/BitcodeReader.cpp

2759 ↗

(On Diff #80976)

What if the value in Record[1] is unexpected?

lib/IR/Verifier.cpp

942 ↗

(On Diff #80976)

The check could be more strict: what if the bitcode contained an invalid ID?

[aaboud]

Thanks for all the reviews and suggestions.
I just need to hear that all of you agree to one of the suggested solutions.

docs/LangRef.rst

4018 ↗

(On Diff #80976)

Once again, I do not mind how we implement it, I just need to hear an agreement from most of the participants in this review.
however, I think that with this approach it might complicate the code in CodeViewDebug, as it needs to parse the checksum string to get the type.

I would like to hear Reid opinion on this suggestion before I go and implement it.

lib/Bitcode/Reader/BitcodeReader.cpp

2759 ↗

(On Diff #80976)

Do you suggest to do this instead?

static_cast<DIChecksum::ChecksumType>(Record[1])

lib/IR/Verifier.cpp

942 ↗

(On Diff #80976)

I am not sure if you are asking about the getTag() or the getType(), so I will answer for both.

1. If we fold the checksum data into the DIFile metadata, then there will be no need for the tag here.
2. Do you suggest that we check for valid range of check sum types? something like this:

(N.getType() >= DIChecksum::FirstValidType && N,getType() <= DIChecksum::LastValidType)

[mehdi_amini]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

Is there already a patch up that shows the CodeViewDebug, so that I can see how it gets more complicated there?
I'd expect it to be a StringSwitch on the prefix in the string instead of a switch on the enum value and that's it.

lib/Bitcode/Reader/BitcodeReader.cpp

2759 ↗

(On Diff #80976)

Not really, this is identical. What I mean is that the value in Record[1] can be totally invalid.

You were concerned about the error checking in the LLParser while it is the least of my concern: the textual IR is more of a "debugging" representation while In Memory and Bitcode representation of the IR are both on the production path.

lib/IR/Verifier.cpp

942 ↗

(On Diff #80976)

Yes, basically N.getType() could be an integer value that is neither None neither one of the valid types.

[aaboud]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

Sorry, I do not have this patch, and I am not planing to.
I hope that Reid can implement this in CodeViewDebug once I commit this patch.
This is why I would like to hear his agreement to your proposal before I go this direction.

lib/Bitcode/Reader/BitcodeReader.cpp

2759 ↗

(On Diff #80976)

1. I implemented the DIChecksumm::CheckSumType similar to DINode::DIFlags.
2. Do you think that if we use String in Record[1], we will be able to validate it here?

I think that it is fare enough to validate these values in the IR, after all LLexer and BitcodeReader are limited on how much they can validate.

[mehdi_amini]

lib/Bitcode/Reader/BitcodeReader.cpp

2759 ↗

(On Diff #80976)

I'm fine validating this in the IR itself, I'd be even fine leaving this field up to the backend which can only ignore and warn if it can't handle the type of the checksum (i.e. the IR verifier would accept null or any string for this field).

I raised it here because you put some effort into having a "robust" textual IR path while the bitcode was not well validated, which seems somehow backward.

[aprantl]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

I was skeptical at first about using a string to represent the checksum, but I came around to agree that this format is likely the most straight-forward way to add this to the IR assembler language.

!DIFile(filename: ..., checksum: !"MD5:deadbeef") // with checksum being an optional nullable field.

We probably still want a DIChecksum wrapper around the string that converts the hex string into an llvm::MD5, etc, and/or stores it as a byte string for a more efficient representation.

include/llvm/IR/DebugInfoMetadata.h

491 ↗

(On Diff #80976)

Why not allow for a nullptr to be passed in that interface?

[rnk]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

I don't think we want a DIChecksum node. If we're concerned about efficiency, the real gain is eliminating the extra allocation.

I'd rather separate the checksum kind out and store it in DIFile::SubclassData32. It's consistent with what we do for DW_VIRTUALITY_*. It only adds minor complexity to the AsmParser, which is not on any critical path, and avoids a StringSwitch in actual code that will run on every compilation.

[mehdi_amini]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

Are you really motivated by the efficiency of a single StringSwitch per DIFile at Codegen time?

The complexity is not much in the AsmParser than in the bitcode (backward compatibility, etc.), the explicit "None" instead of just ignoring the field (null), and the validation in the verifier.

[aprantl]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

@rnk: To make sure I understand where you're going with your comment: You said you would like to separate out the storage of the checksum kind, but how do you propose to store the checksum data?

[aprantl]

About the discussion on distinct: The checksums (and also the DIFile nodes) should be uniqued in an LTO compilation and thus should not be distinct.

[rnk]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

@aprantl I'd store it in an MDString. Different checksums have different lengths, so we'll need a separate allocation for that.

@mehdi_amini What I really care about is that the code to generate this stuff and move it through LLVM is simple. Remember when Duncan encoded as much debug info as strings as possible before moving to the current DI* class hierarchy? Doing stringly-typed things like !"MD5:deadbeef" feels like that, and given a choice, I'd rather avoid that.

Anyway, @aaboud asked for my opinion, so I figured I should spell it out. If you both feel that !"MD5:deadbeef" is better, I don't feel strongly about this and can definitely live with the checksum kind in the string.

[aprantl]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

Would you prefer to store the hexadecimal ASCII encoding in the MDString or the raw bytes?

I would also like to point out that the assembler syntax also doesn't necessarily need to match the in-memory representation. We could use the easy-to-parse !"MD5:deadbeef" syntax while still having a different internal representation.

[mehdi_amini]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

@mehdi_amini What I really care about is that the code to generate this stuff and move it through LLVM is simple

That's also the motivation behind my comment: my impression was that using a MDString is reusing as much of the existing infrastructure and requires changing less places, making it less intrusive for something that likely needs only to be understood manipulated "late" and can stay opaque most of the time. It also makes it handling the checksum as a single unit rather than when it is split.
Not sure if it makes much difference in practice though, that was more my impression of it.

I thought the main motivation for Duncan's work on DI* class was the efficiency of the representation (LTO memory consumption especially). I'd have to ask him though, that is just me speculating.

[aaboud]

Folded Checksum type and data into DIFile metadata.

[rnk]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

I just wanted to echo what Duncan said here about type safety. Ultimately the point of our representation is to have a coherent object model, and I think separating the kind out from the string helps achieve that. The textual IR and bitcode exist to support and serialize our object model.

lib/AsmParser/LLLexer.cpp

812 ↗

(On Diff #81639)

This prefix works for me, but we should ask others to stamp it.

lib/IR/DebugInfoMetadata.cpp

373 ↗

(On Diff #81639)

This assert doesn't look right. The checksum string should be non-empty or null, which is what the isCanonical check does. We shouldn't have an exception for CSK_None.

[mehdi_amini]

docs/LangRef.rst

4018 ↗

(On Diff #80976)

I don't see what can break the "coherency" of the "object model" on a leaf of the graph.

[majnemer]

include/llvm/IR/DebugInfoMetadata.h

478 ↗

(On Diff #81639)

I think it'd be nicer to instead do CSK_Last = CSK_SHA1 and use CSK_Last + 1 instead of CSK_Num. It makes it nicer to switch over the ChecksumKind type.

[aaboud]

Addressed comments by Reid and David.

llvm/trunk/docs/LangRef.rst

 
 DIFile
 """"""
 
 ``DIFile`` nodes represent files. The ``filename:`` can include slashes.
 
 .. code-block:: llvm
 
-    !0 = !DIFile(filename: "path/to/file", directory: "/path/to/dir")
+    !0 = !DIFile(filename: "path/to/file", directory: "/path/to/dir",
+                 checksumkind: CSK_MD5,
+                 checksum: "000102030405060708090a0b0c0d0e0f")
 
 Files are sometimes used in ``scope:`` fields, and are the only valid target
 for ``file:`` fields.
+Valid values for ``checksumkind:`` field are: {CSK_None, CSK_MD5, CSK_SHA1}
 
 .. _DIBasicType:
 
 DIBasicType
 """""""""""
 
 ``DIBasicType`` nodes represent primitive types, such as ``int``, ``bool`` and
 ``float``. ``tag:`` defaults to ``DW_TAG_base_type``.

llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h

enum MetadataCodes {
   METADATA_OLD_NODE = 8,         // OLD_NODE:      [n x (type num, value num)]
   METADATA_OLD_FN_NODE = 9,      // OLD_FN_NODE:   [n x (type num, value num)]
   METADATA_NAMED_NODE = 10,      // NAMED_NODE:    [n x mdnodes]
   METADATA_ATTACHMENT = 11,      // [m x [value, [n x [id, mdnode]]]
   METADATA_GENERIC_DEBUG = 12,   // [distinct, tag, vers, header, n x md num]
   METADATA_SUBRANGE = 13,        // [distinct, count, lo]
   METADATA_ENUMERATOR = 14,      // [distinct, value, name]
   METADATA_BASIC_TYPE = 15,      // [distinct, tag, name, size, align, enc]
-  METADATA_FILE = 16,            // [distinct, filename, directory]
+  METADATA_FILE = 16,            // [distinct, filename, directory, checksumkind, checksum]
   METADATA_DERIVED_TYPE = 17,    // [distinct, ...]
   METADATA_COMPOSITE_TYPE = 18,  // [distinct, ...]
   METADATA_SUBROUTINE_TYPE = 19, // [distinct, flags, types, cc]
   METADATA_COMPILE_UNIT = 20,    // [distinct, ...]
   METADATA_SUBPROGRAM = 21,      // [distinct, ...]
   METADATA_LEXICAL_BLOCK = 22,   // [distinct, scope, file, line, column]
   METADATA_LEXICAL_BLOCK_FILE = 23, //[distinct, scope, file, discriminator]
   METADATA_NAMESPACE = 24,       // [distinct, scope, file, name, line, exportSymbols]

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

public:
     DICompileUnit *
     createCompileUnit(unsigned Lang, DIFile *File, StringRef Producer,
                       bool isOptimized, StringRef Flags, unsigned RV,
                       StringRef SplitName = StringRef(),
                       DICompileUnit::DebugEmissionKind Kind =
                           DICompileUnit::DebugEmissionKind::FullDebug,
                       uint64_t DWOId = 0, bool SplitDebugInlining = true);
 
-    /// Create a file descriptor to hold debugging information
-    /// for a file.
-    DIFile *createFile(StringRef Filename, StringRef Directory);
+    /// Create a file descriptor to hold debugging information for a file.
+    /// \param File     File name.
+    /// \param Dir      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());
 
     /// Create a single enumerator value.
     DIEnumerator *createEnumerator(StringRef Name, int64_t Val);
 
     /// Create a DWARF unspecified type.
     DIBasicType *createUnspecifiedType(StringRef Name);
 
     /// Create C++11 nullptr type.

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

 /// File.
 ///
 /// TODO: Merge with directory/file node (including users).
 /// TODO: Canonicalize paths on creation.
 class DIFile : public DIScope {
   friend class LLVMContextImpl;
   friend class MDNode;
 
-  DIFile(LLVMContext &C, StorageType Storage, ArrayRef<Metadata *> Ops)
-      : DIScope(C, DIFileKind, Storage, dwarf::DW_TAG_file_type, Ops) {}
+public:
+  enum ChecksumKind {
+    CSK_None,
+    CSK_MD5,
+    CSK_SHA1,
+    CSK_Last = CSK_SHA1 // Should be last enumeration.
+  };
+
+private:
+  ChecksumKind CSKind;
+
+  DIFile(LLVMContext &C, StorageType Storage, ChecksumKind CSK,
+         ArrayRef<Metadata *> Ops)
+      : DIScope(C, DIFileKind, Storage, dwarf::DW_TAG_file_type, Ops),
+        CSKind(CSK) {}
   ~DIFile() = default;
 
   static DIFile *getImpl(LLVMContext &Context, StringRef Filename,
-                         StringRef Directory, StorageType Storage,
-                         bool ShouldCreate = true) {
+                         StringRef Directory, ChecksumKind CSK, StringRef CS,
+                         StorageType Storage, bool ShouldCreate = true) {
     return getImpl(Context, getCanonicalMDString(Context, Filename),
-                   getCanonicalMDString(Context, Directory), Storage,
-                   ShouldCreate);
+                   getCanonicalMDString(Context, Directory), CSK,
+                   getCanonicalMDString(Context, CS), Storage, ShouldCreate);
   }
   static DIFile *getImpl(LLVMContext &Context, MDString *Filename,
-                         MDString *Directory, StorageType Storage,
-                         bool ShouldCreate = true);
+                         MDString *Directory, ChecksumKind CSK, MDString *CS,
+                         StorageType Storage, bool ShouldCreate = true);
 
   TempDIFile cloneImpl() const {
-    return getTemporary(getContext(), getFilename(), getDirectory());
+    return getTemporary(getContext(), getFilename(), getDirectory(),
+                        getChecksumKind(), getChecksum());
   }
 
 public:
-  DEFINE_MDNODE_GET(DIFile, (StringRef Filename, StringRef Directory),
-                    (Filename, Directory))
-  DEFINE_MDNODE_GET(DIFile, (MDString * Filename, MDString *Directory),
-                    (Filename, Directory))
+  DEFINE_MDNODE_GET(DIFile, (StringRef Filename, StringRef Directory,
+                             ChecksumKind CSK = CSK_None,
+                             StringRef CS = StringRef()),
+                    (Filename, Directory, CSK, CS))
+  DEFINE_MDNODE_GET(DIFile, (MDString *Filename, MDString *Directory,
+                             ChecksumKind CSK = CSK_None,
+                             MDString *CS = nullptr),
+                    (Filename, Directory, CSK, 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;
 
   MDString *getRawFilename() const { return getOperandAs<MDString>(0); }
   MDString *getRawDirectory() const { return getOperandAs<MDString>(1); }
+  MDString *getRawChecksum() const { return getOperandAs<MDString>(2); }
+
+  static 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/LLLexer.cpp

#define DWKEYWORD(TYPE, TOKEN)                                                 \
   DWKEYWORD(MACINFO, DwarfMacinfo);
 
 #undef DWKEYWORD
 
   if (Keyword.startswith("DIFlag")) {
     StrVal.assign(Keyword.begin(), Keyword.end());
     return lltok::DIFlag;
   }
+
+  if (Keyword.startswith("CSK_")) {
+    StrVal.assign(Keyword.begin(), Keyword.end());
+    return lltok::ChecksumKind;
+  }
+
   if (Keyword == "NoDebug" || Keyword == "FullDebug" ||
       Keyword == "LineTablesOnly") {
     StrVal.assign(Keyword.begin(), Keyword.end());
     return lltok::EmissionKind;
   }
 
   // Check for [us]0x[0-9A-Fa-f]+ which are Hexadecimal constant generated by
   // the CFE to avoid forcing it to deal with 64-bit numbers.

llvm/trunk/lib/AsmParser/LLParser.cpp

struct MDStringField : public MDFieldImpl<MDString *> {
   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) {}
+};
+
 } // end anonymous namespace
 
 namespace llvm {
 
 template <>
 bool LLParser::ParseMDField(LocTy Loc, StringRef Name,
                             MDUnsignedField &Result) {
   if (Lex.getKind() != lltok::APSInt || Lex.getAPSIntVal().isSigned())
bool LLParser::ParseMDField(LocTy Loc, StringRef Name, MDFieldList &Result) {
   SmallVector<Metadata *, 4> MDs;
   if (ParseMDNodeVector(MDs))
     return true;
 
   Result.assign(std::move(MDs));
   return false;
 }
 
+template <>
+bool LLParser::ParseMDField(LocTy Loc, StringRef Name,
+                            ChecksumKindField &Result) {
+  if (Lex.getKind() != lltok::ChecksumKind)
+    return TokError(
+        "invalid checksum kind" + Twine(" '") + Lex.getStrVal() + "'");
+
+  DIFile::ChecksumKind CSKind = DIFile::getChecksumKind(Lex.getStrVal());
+
+  Result.assign(CSKind);
+  Lex.Lex();
+  return false;
+}
+
 } // end namespace llvm
 
 template <class ParserTy>
 bool LLParser::ParseMDFieldsImplBody(ParserTy parseField) {
   do {
     if (Lex.getKind() != lltok::LabelStr)
       return TokError("expected field label here");
 

 #undef VISIT_MD_FIELDS
 
   Result = GET_OR_DISTINCT(DISubroutineType,
                            (Context, flags.Val, cc.Val, types.Val));
   return false;
 }
 
 /// ParseDIFileType:
-///   ::= !DIFileType(filename: "path/to/file", directory: "/path/to/dir")
+///   ::= !DIFileType(filename: "path/to/file", directory: "/path/to/dir"
+///                   checksumkind: CSK_MD5,
+///                   checksum: "000102030405060708090a0b0c0d0e0f")
 bool LLParser::ParseDIFile(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   REQUIRED(filename, MDStringField, );                                         \
-  REQUIRED(directory, MDStringField, );
+  REQUIRED(directory, MDStringField, );                                        \
+  OPTIONAL(checksumkind, ChecksumKindField, );                                 \
+  OPTIONAL(checksum, MDStringField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
-  Result = GET_OR_DISTINCT(DIFile, (Context, filename.Val, directory.Val));
+  Result = GET_OR_DISTINCT(DIFile, (Context, filename.Val, directory.Val,
+                                    checksumkind.Val, checksum.Val));
   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/AsmParser/LLToken.h

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

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

MetadataList.assignValue(
                         (Context, getMDOrNull(Record[1]),
                          getMDString(Record[2]), getMDString(Record[3]),
                          getMDString(Record[4]), getMDString(Record[5]))),
         NextMetadataNo++);
     break;
   }
 
   case bitc::METADATA_FILE: {
-    if (Record.size() != 3)
+    if (Record.size() != 3 && Record.size() != 5)
       return error("Invalid record");
 
     IsDistinct = Record[0];
     MetadataList.assignValue(
         GET_OR_DISTINCT(
-            DIFile, (Context, getMDString(Record[1]), getMDString(Record[2]))),
+            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]))),
         NextMetadataNo++);
     break;
   }
   case bitc::METADATA_COMPILE_UNIT: {
     if (Record.size() < 14 || Record.size() > 17)
       return error("Invalid record");
 
     // Ignore Record[0], which indicates whether this compile unit is

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()));
 
   Stream.EmitRecord(bitc::METADATA_FILE, Record, Abbrev);
   Record.clear();
 }
 
 void ModuleBitcodeWriter::writeDICompileUnit(const DICompileUnit *N,
                                              SmallVectorImpl<uint64_t> &Record,
                                              unsigned Abbrev) {

llvm/trunk/lib/IR/AsmWriter.cpp

struct MDFieldPrinter {
   explicit MDFieldPrinter(raw_ostream &Out)
       : Out(Out), TypePrinter(nullptr), Machine(nullptr), Context(nullptr) {}
   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 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::printString(StringRef Name, StringRef Value,
                                  bool ShouldSkipEmpty) {
   if (ShouldSkipEmpty && Value.empty())
     return;
 
   Out << FS << Name << ": \"";
   PrintEscapedString(Value, Out);
   Out << "\"";

 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);
   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,
                                                        unsigned Line,
                                                        StringRef Name) {
   // 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, Line, Name, AllImportedModules);
 }
 
-DIFile *DIBuilder::createFile(StringRef Filename, StringRef Directory) {
-  return DIFile::get(VMContext, Filename, Directory);
+DIFile *DIBuilder::createFile(StringRef Filename, StringRef Directory,
+                              DIFile::ChecksumKind CSKind, StringRef Checksum) {
+  return DIFile::get(VMContext, Filename, Directory, CSKind, Checksum);
 }
 
 DIEnumerator *DIBuilder::createEnumerator(StringRef Name, int64_t Val) {
   assert(!Name.empty() && "Unable to create enumerator without name");
   return DIEnumerator::get(VMContext, Val, Name);
 }
 
 DIBasicType *DIBuilder::createUnspecifiedType(StringRef Name) {

llvm/trunk/lib/IR/DebugInfoMetadata.cpp

DISubroutineType *DISubroutineType::getImpl(LLVMContext &Context, DIFlags Flags,
                                             uint8_t CC, Metadata *TypeArray,
                                             StorageType Storage,
                                             bool ShouldCreate) {
   DEFINE_GETIMPL_LOOKUP(DISubroutineType, (Flags, CC, TypeArray));
   Metadata *Ops[] = {nullptr, nullptr, nullptr, TypeArray};
   DEFINE_GETIMPL_STORE(DISubroutineType, (Flags, CC), Ops);
 }
 
+static const char *ChecksumKindName[DIFile::CSK_Last + 1] = {
+  "CSK_None",
+  "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() const {
+  assert(CSKind <= DIFile::CSK_Last && "Invalid checksum kind");
+  return ChecksumKindName[CSKind];
+}
+
 DIFile *DIFile::getImpl(LLVMContext &Context, MDString *Filename,
-                        MDString *Directory, StorageType Storage,
+                        MDString *Directory, DIFile::ChecksumKind CSKind,
+                        MDString *Checksum, StorageType Storage,
                         bool ShouldCreate) {
   assert(isCanonical(Filename) && "Expected canonical MDString");
   assert(isCanonical(Directory) && "Expected canonical MDString");
-  DEFINE_GETIMPL_LOOKUP(DIFile, (Filename, Directory));
-  Metadata *Ops[] = {Filename, Directory};
-  DEFINE_GETIMPL_STORE_NO_CONSTRUCTOR_ARGS(DIFile, Ops);
+  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);
 }
 
 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,

llvm/trunk/lib/IR/LLVMContextImpl.h

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

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_None ||
+            N.getChecksum().empty()), "invalid checksum kind", &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.

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}
-!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}
+; 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, !33}
+!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}
 
 ; 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: !28 = !DIMacro(type: DW_MACINFO_define, line: 9, name: "Name", value: "Value")
 ; CHECK-NEXT: !29 = distinct !{!28}
 ; 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: "")
 No newline at end of file

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";
-  auto *N = DIFile::get(Context, Filename, Directory);
+  DIFile::ChecksumKind CSKind = DIFile::CSK_MD5;
+  StringRef Checksum = "000102030405060708090a0b0c0d0e0f";
+  auto *N = DIFile::get(Context, Filename, Directory, CSKind, Checksum);
 
   EXPECT_EQ(dwarf::DW_TAG_file_type, N->getTag());
   EXPECT_EQ(Filename, N->getFilename());
   EXPECT_EQ(Directory, N->getDirectory());
-  EXPECT_EQ(N, DIFile::get(Context, Filename, Directory));
-
-  EXPECT_NE(N, DIFile::get(Context, "other", Directory));
-  EXPECT_NE(N, DIFile::get(Context, Filename, "other"));
+  EXPECT_EQ(CSKind, N->getChecksumKind());
+  EXPECT_EQ(Checksum, N->getChecksum());
+  EXPECT_EQ(N, DIFile::get(Context, Filename, Directory, CSKind, 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, Filename, Directory, DIFile::CSK_SHA1, Checksum));
+  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.
   DIScope *N = DIFile::get(Context, "file", "dir");