[PATCH] D27642: DebugInfo: Added support for Checksum debug info feature (LLVM IR part)

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

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



================
Comment at: docs/LangRef.rst:4018
+                     data: "000102030405060708090a0b0c0d0e0f")
+
 .. _DIBasicType:
----------------
aaboud wrote:
> mehdi_amini wrote:
> > 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)
> 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.
> 
> 
> 
> 
> 
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.


================
Comment at: include/llvm/IR/DebugInfoMetadata.h:430
   inline StringRef getDirectory() const;
+  inline DIChecksum *getChecksum() const;
 
----------------
aaboud wrote:
> mehdi_amini wrote:
> > Why there? Where is this API used?
> > 
> We need to getChecksum from DICompileUnit, however, this information is stored in the DIScope.
I don't believe we really need it. We can probably do `CU->getFile()->getChecksum()`.


================
Comment at: lib/IR/DIBuilder.cpp:127-128
+      VMContext, Lang,
+      DIFile::get(VMContext, Filename, Directory,
+                  DIChecksum::get(VMContext, ChecksumType, Checksum)),
+      Producer, isOptimized, Flags, RunTimeVer, SplitName, Kind, nullptr,
----------------
Instead of having createCompileUnit take Filename, Directory, ChecksumType, and Checksum, have it take a DIFIle.


https://reviews.llvm.org/D27642