[IR] Add a boolean field in DILocation to know if a line must covered or not

[calixte]

Summary

Some lines have a hit counter where they should not have one.
For example, in C++, some cleanup is adding at the end of a scope represented by a '}'.
So such a line has a hit counter where a user expects to not have one.
The goal of the patch is to add this information in DILocation which is used to get the covered lines in GCOVProfiling.cpp.
A following patch in clang will add this information when generating IR (https://reviews.llvm.org/D49916).

[vsk]

Some lines have a hit counter where they should not have one.
For example, in C++, some cleanup is adding at the end of a scope represented by a '}'.
So such a line has a hit counter where a user expects to not have one.

Could you share an example of this resulting in an incorrect coverage report?

The goal of the patch is to add this information in DILocation which is used to get the covered lines in GCOVProfiling.cpp.

This requires adding an operand to DILocation, which is a fairly expensive thing to do. Have you considered any alternatives?

A following patch in clang will add this information when generating IR (https://reviews.llvm.org/D49916).

[marco-c]

In D49915#1180453, @vsk wrote:

Some lines have a hit counter where they should not have one.
For example, in C++, some cleanup is adding at the end of a scope represented by a '}'.
So such a line has a hit counter where a user expects to not have one.

Could you share an example of this resulting in an incorrect coverage report?

There are some examples in the tests that are fixed here: https://reviews.llvm.org/D49917.

The goal of the patch is to add this information in DILocation which is used to get the covered lines in GCOVProfiling.cpp.

This requires adding an operand to DILocation, which is a fairly expensive thing to do. Have you considered any alternatives?

Both me and Calixte had reservations about adding more data to DILocation too, but we couldn't find an alternative. It's possible that there might be some alternative we can't figure out due to our lack of experience with LLVM.
One thing I tried is not assigning lines to instructions that are TerminatorInst and !ReturnInst, but I think this causes us to skip some lines that we don't want to skip (e.g. the break clause in a switch).

[vsk]

In D49915#1180515, @marco-c wrote:

In D49915#1180453, @vsk wrote:

Some lines have a hit counter where they should not have one.
For example, in C++, some cleanup is adding at the end of a scope represented by a '}'.
So such a line has a hit counter where a user expects to not have one.

Could you share an example of this resulting in an incorrect coverage report?

There are some examples in the tests that are fixed here: https://reviews.llvm.org/D49917.

I see, thanks.

The goal of the patch is to add this information in DILocation which is used to get the covered lines in GCOVProfiling.cpp.

This requires adding an operand to DILocation, which is a fairly expensive thing to do. Have you considered any alternatives?

Both me and Calixte had reservations about adding more data to DILocation too, but we couldn't find an alternative. It's possible that there might be some alternative we can't figure out due to our lack of experience with LLVM.
One thing I tried is not assigning lines to instructions that are TerminatorInst and !ReturnInst, but I think this causes us to skip some lines that we don't want to skip (e.g. the break clause in a switch).

Perhaps you could borrow the high bit from the discriminator operand?

[dblaikie]

This looks problematic - seems suspicious that the covered value is a bool member of DILocation, but nothing else is - everything else is stored in the metadata proper. Is there a good reason for that? (it provably doesn't need to be serialized? Could this be stored as side-data in a map in an analysis instead?)

I couldn't quite spot it - where's the code that sets or constructs a DILocation with covered = true?

[marco-c]

In D49915#1181637, @dblaikie wrote:

I couldn't quite spot it - where's the code that sets or constructs a DILocation with covered = true?

https://reviews.llvm.org/D49916

BTW, I'd probably rename covered to coverable to make it more clear.

[rnk]

I agree, this patch needs serialization through bitcode and IR before it can land.

DILocation isn't only used for coverage, I think this should probably be a more general purpose flag, like "isImplicitCode", to indicate that the code was generated as part of a destructor cleanup, an epilogue, or something else that requires the compiler to generate IR that isn't obviously attributed to user source code. Then GCOV can decide for itself how it wants to generate coverage.

[aprantl]

DILocation isn't only used for coverage, I think this should probably be a more general purpose flag, like "isImplicitCode", to indicate that the code was generated as part of a destructor cleanup, an epilogue, or something else that requires the compiler to generate IR that isn't obviously attributed to user source code. Then GCOV can decide for itself how it wants to generate coverage.

We have a marker for compiler-generated code that has no relation to any source code; it's the special line number 0. This sounds like this patch is trying to work around a frontend bug, and really the frontend shouldn't stick misleading line numbers on the code? There's an RAII object in clang to generate locations for compiler/auto-generated code.

[vsk]

In D49915#1184336, @aprantl wrote:

DILocation isn't only used for coverage, I think this should probably be a more general purpose flag, like "isImplicitCode", to indicate that the code was generated as part of a destructor cleanup, an epilogue, or something else that requires the compiler to generate IR that isn't obviously attributed to user source code. Then GCOV can decide for itself how it wants to generate coverage.

We have a marker for compiler-generated code that has no relation to any source code; it's the special line number 0. This sounds like this patch is trying to work around a frontend bug, and really the frontend shouldn't stick misleading line numbers on the code? There's an RAII object in clang to generate locations for compiler/auto-generated code.

A caveat to this: plenty of users (and plenty of tests) expect to be able to set breakpoints on scope-closing curly braces. Assigning line 0 to those braces would at the minimum break a lot of tests, and would likely be viewed as a serious regression.

[rnk]

In D49915#1184440, @vsk wrote:

A caveat to this: plenty of users (and plenty of tests) expect to be able to set breakpoints on scope-closing curly braces. Assigning line 0 to those braces would at the minimum break a lot of tests, and would likely be viewed as a serious regression.

Right. The frontend should try to encode enough information that the various consumers of DILocations can decide for themselves what tables they should emit. Our line tables should probably stay as they are now, and the various code coverage and instr profiling implementations can do whatever they think is appropriate with this information. With that in mind, if we're going to grow DILocation, what we really want is probably a set of location flags, if we're going to grow DILocation.

[vsk]

In D49915#1184574, @rnk wrote:

In D49915#1184440, @vsk wrote:

A caveat to this: plenty of users (and plenty of tests) expect to be able to set breakpoints on scope-closing curly braces. Assigning line 0 to those braces would at the minimum break a lot of tests, and would likely be viewed as a serious regression.

Right. The frontend should try to encode enough information that the various consumers of DILocations can decide for themselves what tables they should emit. Our line tables should probably stay as they are now, and the various code coverage and instr profiling implementations can do whatever they think is appropriate with this information. With that in mind, if we're going to grow DILocation, what we really want is probably a set of location flags, if we're going to grow DILocation.

Yes, c.f the source-based coverage instrumentation which ended up needing two bits of information from the frontend per segment: http://llvm.org/doxygen/structllvm_1_1coverage_1_1CoverageSegment.html.

(Btw I'm not convinced we have to grow DILocation; I'm not aware of the use case of enabling SamplePGO along with GCOV instrumentation, so I don't see why it's not feasible to borrow 1-2 bits from the discriminator operand. + @xur & @danielcdh for any corrections here)

[rnk]

In D49915#1184590, @vsk wrote:

In D49915#1184574, @rnk wrote:

In D49915#1184440, @vsk wrote:

A caveat to this: plenty of users (and plenty of tests) expect to be able to set breakpoints on scope-closing curly braces. Assigning line 0 to those braces would at the minimum break a lot of tests, and would likely be viewed as a serious regression.

Right. The frontend should try to encode enough information that the various consumers of DILocations can decide for themselves what tables they should emit. Our line tables should probably stay as they are now, and the various code coverage and instr profiling implementations can do whatever they think is appropriate with this information. With that in mind, if we're going to grow DILocation, what we really want is probably a set of location flags, if we're going to grow DILocation.

Yes, c.f the source-based coverage instrumentation which ended up needing two bits of information from the frontend per segment: http://llvm.org/doxygen/structllvm_1_1coverage_1_1CoverageSegment.html.

(Btw I'm not convinced we have to grow DILocation; I'm not aware of the use case of enabling SamplePGO along with GCOV instrumentation, so I don't see why it's not feasible to borrow 1-2 bits from the discriminator operand. + @xur & @danielcdh for any corrections here)

The discriminator seems to be stored in a separate parent scope, though. I'm not sure that's really the right place to store this bit.

[calixte]

DILocation is a subclass of MDNode which is itself a subclass of Metadata. And in Metadata there is "unsigned char Storage" which uses only 2 bits so 6 bits are available here.
Maybe I could add "unsigned char ImplicitCode : 1" and "unsigned char Storage: 7" in class Metadata and so don't increase the DILocation size.
What do you think ?

[calixte]

• Use an available bit in Metadata to not increase the memory use
• Update Writer/Reader for IR and Bitcode to take into account the new flag

[calixte]

ping ?

[calixte]

• Fix an error in MetadataLoader.cpp
• Update tests to take into account the new added field

[aprantl]

include/llvm/IR/DebugInfoMetadata.h

1469 ↗

(On Diff #164634)

For the casual reader it will be entirely non-obvious what "implicit" code means. Can you add the explanation that you added at the top of the review to the doxygen comments (and perhaps to SourceLevelDebugging.rst ?

lib/IR/AsmWriter.cpp

1756 ↗

(On Diff #164634)

It would be better to match the Parser and only print this if it deviates from the default value.

[calixte]

• AsmWriter.cpp: print isImplicitCode only when it's true
• Improve documentation for the new added flag

[rnk]

Needs tests, but I'm on board with this and the implementation.

docs/SourceLevelDebugging.rst

436 ↗

(On Diff #164827)

"a real code" isn't very descriptive. Consider "doesn't correspond to source code written by the user".

446 ↗

(On Diff #164827)

"explicitly"

include/llvm/IR/DebugInfoMetadata.h

1469 ↗

(On Diff #164827)

Code is a weird word, I believe it's more correct to say "... corresponds to implicit code."

include/llvm/IR/Metadata.h

69–72 ↗

(On Diff #164827)

This will not achieve the layout you want with MSVC. I would recommend using uint8_t for both to ensure they are packed.

[calixte]

• Add a test: test/Bitcode/DILocation-implicit-code.ll
• and update: test/Assembler/dilocation.ll
• fix layout issue with MSVC
• fix the doc

[rnk]

Thanks, I think this looks good with a minor tweak to the test.

test/Bitcode/DILocation-implicit-code.ll

169–170 ↗

(On Diff #166003)

I think it's best if you don't comment out the original DILocation metadata so that someone can easily regenerate the .bc file. Add a separate, new CHECK line. You should also ignore irrelevant !N markers if you can. Something like this is best, since llvm-dis could add some auto-upgrading that disrupts the metadata numbering:

; CHECK: !DILocation(line: 20, column: 1, scope: !{{[0-9]+}}, isImplicitCode: true)
; CHECK: !DILocation(line: 17, column: 5, scope: !{{[0-9]+}}, isImplicitCode: true)

[calixte]

• Update test DILocation-implicit-code.ll

[calixte]

• Fix .bc for test

[vsk]

This caused a backwards-compatibility issue which made llvm-dis crash when loading old bitcode (see https://reviews.llvm.org/rL342631#inline-2294). I've committed a fix in r342678 to get our bots back to a healthy state. I'm open to any post-commit review -- while I don't anticipate there being an issue, if there is one I'd rather iterate on a passing build.

llvm/trunk/docs/SourceLevelDebugging.rst

 ``Instruction::getDebugLoc()`` and ``DILocation::getLine()``.
 
 .. code-block:: c++
 
   if (DILocation *Loc = I->getDebugLoc()) { // Here I is an LLVM instruction
     unsigned Line = Loc->getLine();
     StringRef File = Loc->getFilename();
     StringRef Dir = Loc->getDirectory();
+    bool ImplicitCode = Loc->isImplicitCode();
   }
 
+When the flag ImplicitCode is true then it means that the Instruction has been
+added by the front-end but doesn't correspond to source code written by the user. For example
+
+.. code-block:: c++
+
+  if (MyBoolean) {
+    MyObject MO;
+    ...
+  }
+
+At the end of the scope the MyObject's destructor is called but it isn't written
+explicitly. This information is useful to avoid to have counters on brackets when
+making code coverage.
+
 C/C++ global variable information
 ---------------------------------
 
 Given an integer global variable declared as follows:
 
 .. code-block:: c
 
   _Alignas(8) int MyGlobal = 100;

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

 /// Debug location.
 ///
 /// A debug location in source code, used for debug info and otherwise.
 class DILocation : public MDNode {
   friend class LLVMContextImpl;
   friend class MDNode;
 
   DILocation(LLVMContext &C, StorageType Storage, unsigned Line,
-             unsigned Column, ArrayRef<Metadata *> MDs);
+             unsigned Column, ArrayRef<Metadata *> MDs, bool ImplicitCode);
   ~DILocation() { dropAllReferences(); }
 
   static DILocation *getImpl(LLVMContext &Context, unsigned Line,
                              unsigned Column, Metadata *Scope,
-                             Metadata *InlinedAt, StorageType Storage,
-                             bool ShouldCreate = true);
+                             Metadata *InlinedAt, bool ImplicitCode,
+                             StorageType Storage, bool ShouldCreate = true);
   static DILocation *getImpl(LLVMContext &Context, unsigned Line,
                              unsigned Column, DILocalScope *Scope,
-                             DILocation *InlinedAt, StorageType Storage,
-                             bool ShouldCreate = true) {
+                             DILocation *InlinedAt, bool ImplicitCode,
+                             StorageType Storage, bool ShouldCreate = true) {
     return getImpl(Context, Line, Column, static_cast<Metadata *>(Scope),
-                   static_cast<Metadata *>(InlinedAt), Storage, ShouldCreate);
+                   static_cast<Metadata *>(InlinedAt), ImplicitCode, Storage,
+                   ShouldCreate);
   }
 
   /// With a given unsigned int \p U, use up to 13 bits to represent it.
   /// old_bit 1~5  --> new_bit 1~5
   /// old_bit 6~12 --> new_bit 7~13
   /// new_bit_6 is 0 if higher bits (7~13) are all 0
   static unsigned getPrefixEncodingFromUnsigned(unsigned U) {
     U &= 0xfff;
static unsigned getNextComponentInDiscriminator(unsigned D) {
     else
       return D >> 1;
   }
 
   TempDILocation cloneImpl() const {
     // Get the raw scope/inlinedAt since it is possible to invoke this on
     // a DILocation containing temporary metadata.
     return getTemporary(getContext(), getLine(), getColumn(), getRawScope(),
-                        getRawInlinedAt());
+                        getRawInlinedAt(), isImplicitCode());
   }
 
 public:
   // Disallow replacing operands.
   void replaceOperandWith(unsigned I, Metadata *New) = delete;
 
   DEFINE_MDNODE_GET(DILocation,
                     (unsigned Line, unsigned Column, Metadata *Scope,
-                     Metadata *InlinedAt = nullptr),
-                    (Line, Column, Scope, InlinedAt))
+                     Metadata *InlinedAt = nullptr, bool ImplicitCode = false),
+                    (Line, Column, Scope, InlinedAt, ImplicitCode))
   DEFINE_MDNODE_GET(DILocation,
                     (unsigned Line, unsigned Column, DILocalScope *Scope,
-                     DILocation *InlinedAt = nullptr),
-                    (Line, Column, Scope, InlinedAt))
+                     DILocation *InlinedAt = nullptr,
+                     bool ImplicitCode = false),
+                    (Line, Column, Scope, InlinedAt, ImplicitCode))
 
   /// Return a (temporary) clone of this.
   TempDILocation clone() const { return cloneImpl(); }
 
   unsigned getLine() const { return SubclassData32; }
   unsigned getColumn() const { return SubclassData16; }
   DILocalScope *getScope() const { return cast<DILocalScope>(getRawScope()); }
 
   DILocation *getInlinedAt() const {
     return cast_or_null<DILocation>(getRawInlinedAt());
   }
 
+  /// Check if the location corresponds to an implicit code.
+  /// When the ImplicitCode flag is true, it means that the Instruction
+  /// with this DILocation has been added by the front-end but it hasn't been
+  /// written explicitly by the user (e.g. cleanup stuff in C++ put on a closing
+  /// bracket). It's useful for code coverage to not show a counter on "empty"
+  /// lines.
+  bool isImplicitCode() const { return ImplicitCode; }
+  void setImplicitCode(bool ImplicitCode) { this->ImplicitCode = ImplicitCode; }
+
   DIFile *getFile() const { return getScope()->getFile(); }
   StringRef getFilename() const { return getScope()->getFilename(); }
   StringRef getDirectory() const { return getScope()->getDirectory(); }
   Optional<StringRef> getSource() const { return getScope()->getSource(); }
 
   /// Get the scope where this is inlined.
   ///
   /// Walk through \a getInlinedAt() and return \a getScope() from the deepest

llvm/trunk/include/llvm/IR/DebugLoc.h

public:
     ///
     /// Create a new DebugLoc at the specified line/col and scope/inline.  This
     /// forwards to \a DILocation::get().
     ///
     /// If \c !Scope, returns a default-constructed \a DebugLoc.
     ///
     /// FIXME: Remove this.  Users should use DILocation::get().
     static DebugLoc get(unsigned Line, unsigned Col, const MDNode *Scope,
-                        const MDNode *InlinedAt = nullptr);
+                        const MDNode *InlinedAt = nullptr,
+                        bool ImplicitCode = false);
 
     enum { ReplaceLastInlinedAt = true };
     /// Rebuild the entire inlined-at chain for this instruction so that the top of
     /// the chain now is inlined-at the new call site.
     /// \param   InlinedAt    The new outermost inlined-at in the chain.
     /// \param   ReplaceLast  Replace the last location in the inlined-at chain.
     static DebugLoc appendInlinedAt(DebugLoc DL, DILocation *InlinedAt,
                                     LLVMContext &Ctx,
public:
     ///
     /// FIXME: Remove this.  Users should use DILocation/DILocalScope API to
     /// find the subprogram, and then DILocation::get().
     DebugLoc getFnDebugLoc() const;
 
     /// Return \c this as a bar \a MDNode.
     MDNode *getAsMDNode() const { return Loc; }
 
+    /// Check if the DebugLoc corresponds to an implicit code.
+    bool isImplicitCode() const;
+    void setImplicitCode(bool ImplicitCode);
+
     bool operator==(const DebugLoc &DL) const { return Loc == DL.Loc; }
     bool operator!=(const DebugLoc &DL) const { return Loc != DL.Loc; }
 
     void dump() const;
 
     /// prints source location /path/to/file.exe:line:col @[inlined at]
     void print(raw_ostream &OS) const;
   };
 
 } // end namespace llvm
 
 #endif /* LLVM_SUPPORT_DEBUGLOC_H */

llvm/trunk/include/llvm/IR/Metadata.h

class Metadata {
   /// RTTI.
   const unsigned char SubclassID;
 
 protected:
   /// Active type of storage.
   enum StorageType { Uniqued, Distinct, Temporary };
 
   /// Storage flag for non-uniqued, otherwise unowned, metadata.
-  unsigned char Storage;
+  unsigned char Storage : 7;
   // TODO: expose remaining bits to subclasses.
 
+  unsigned char ImplicitCode : 1;
+
   unsigned short SubclassData16 = 0;
   unsigned SubclassData32 = 0;
 
 public:
   enum MetadataKind {
 #define HANDLE_METADATA_LEAF(CLASS) CLASS##Kind,
 #include "llvm/IR/Metadata.def"
   };
 
 protected:
   Metadata(unsigned ID, StorageType Storage)
-      : SubclassID(ID), Storage(Storage) {
+      : SubclassID(ID), Storage(Storage), ImplicitCode(false) {
     static_assert(sizeof(*this) == 8, "Metadata fields poorly packed");
   }
 
   ~Metadata() = default;
 
   /// Default handling of a changed operand, which asserts.
   ///
   /// If subclasses pass themselves in as owners to a tracking node reference,

llvm/trunk/lib/AsmParser/LLParser.cpp

do {                                                                         \
     }, ClosingLoc))                                                            \
       return true;                                                             \
     VISIT_MD_FIELDS(NOP_FIELD, REQUIRE_FIELD)                                  \
   } while (false)
 #define GET_OR_DISTINCT(CLASS, ARGS)                                           \
   (IsDistinct ? CLASS::getDistinct ARGS : CLASS::get ARGS)
 
 /// ParseDILocationFields:
-///   ::= !DILocation(line: 43, column: 8, scope: !5, inlinedAt: !6)
+///   ::= !DILocation(line: 43, column: 8, scope: !5, inlinedAt: !6,
+///   isImplicitCode: true)
 bool LLParser::ParseDILocation(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   OPTIONAL(line, LineField, );                                                 \
   OPTIONAL(column, ColumnField, );                                             \
   REQUIRED(scope, MDField, (/* AllowNull */ false));                           \
-  OPTIONAL(inlinedAt, MDField, );
+  OPTIONAL(inlinedAt, MDField, );                                              \
+  OPTIONAL(isImplicitCode, MDBoolField, (false));
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
-  Result = GET_OR_DISTINCT(
-      DILocation, (Context, line.Val, column.Val, scope.Val, inlinedAt.Val));
+  Result =
+      GET_OR_DISTINCT(DILocation, (Context, line.Val, column.Val, scope.Val,
+                                   inlinedAt.Val, isImplicitCode.Val));
   return false;
 }
 
 /// ParseGenericDINode:
 ///   ::= !GenericDINode(tag: 15, header: "...", operands: {...})
 bool LLParser::ParseGenericDINode(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   REQUIRED(tag, DwarfTagField, );                                              \

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

while (true) {
 
     case bitc::FUNC_CODE_DEBUG_LOC: {      // DEBUG_LOC: [line, col, scope, ia]
       I = getLastInstruction();
       if (!I || Record.size() < 4)
         return error("Invalid record");
 
       unsigned Line = Record[0], Col = Record[1];
       unsigned ScopeID = Record[2], IAID = Record[3];
+      bool isImplicitCode = Record.size() == 5 && Record[4];
 
       MDNode *Scope = nullptr, *IA = nullptr;
       if (ScopeID) {
         Scope = MDLoader->getMDNodeFwdRefOrNull(ScopeID - 1);
         if (!Scope)
           return error("Invalid record");
       }
       if (IAID) {
         IA = MDLoader->getMDNodeFwdRefOrNull(IAID - 1);
         if (!IA)
           return error("Invalid record");
       }
-      LastLoc = DebugLoc::get(Line, Col, Scope, IA);
+      LastLoc = DebugLoc::get(Line, Col, Scope, IA, isImplicitCode);
       I->setDebugLoc(LastLoc);
       I = nullptr;
       continue;
     }
 
     case bitc::FUNC_CODE_INST_BINOP: {    // BINOP: [opval, ty, opval, opcode]
       unsigned OpNum = 0;
       Value *LHS, *RHS;

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

for (unsigned ID : Record)
       Elts.push_back(getMDOrNull(ID));
     MetadataList.assignValue(IsDistinct ? MDNode::getDistinct(Context, Elts)
                                         : MDNode::get(Context, Elts),
                              NextMetadataNo);
     NextMetadataNo++;
     break;
   }
   case bitc::METADATA_LOCATION: {
-    if (Record.size() != 5)
+    if (Record.size() != 6)
       return error("Invalid record");
 
     IsDistinct = Record[0];
     unsigned Line = Record[1];
     unsigned Column = Record[2];
     Metadata *Scope = getMD(Record[3]);
     Metadata *InlinedAt = getMDOrNull(Record[4]);
+    bool ImplicitCode = Record[5];
     MetadataList.assignValue(
-        GET_OR_DISTINCT(DILocation, (Context, Line, Column, Scope, InlinedAt)),
+        GET_OR_DISTINCT(DILocation, (Context, Line, Column, Scope, InlinedAt,
+                                     ImplicitCode)),
         NextMetadataNo);
     NextMetadataNo++;
     break;
   }
   case bitc::METADATA_GENERIC_DEBUG: {
     if (Record.size() < 4)
       return error("Invalid record");
 

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

unsigned ModuleBitcodeWriter::createDILocationAbbrev() {
   // location (it's never more expensive than building an array size 1).
   auto Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::METADATA_LOCATION));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1));
   return Stream.EmitAbbrev(std::move(Abbv));
 }
 
 void ModuleBitcodeWriter::writeDILocation(const DILocation *N,
                                           SmallVectorImpl<uint64_t> &Record,
                                           unsigned &Abbrev) {
   if (!Abbrev)
     Abbrev = createDILocationAbbrev();
 
   Record.push_back(N->isDistinct());
   Record.push_back(N->getLine());
   Record.push_back(N->getColumn());
   Record.push_back(VE.getMetadataID(N->getScope()));
   Record.push_back(VE.getMetadataOrNullID(N->getInlinedAt()));
+  Record.push_back(N->isImplicitCode());
 
   Stream.EmitRecord(bitc::METADATA_LOCATION, Record, Abbrev);
   Record.clear();
 }
 
 unsigned ModuleBitcodeWriter::createGenericDINodeAbbrev() {
   // Assume the column is usually under 128, and always output the inlined-at
   // location (it's never more expensive than building an array size 1).
for (BasicBlock::const_iterator I = BB->begin(), E = BB->end();
         Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_LOC_AGAIN, Vals);
         continue;
       }
 
       Vals.push_back(DL->getLine());
       Vals.push_back(DL->getColumn());
       Vals.push_back(VE.getMetadataOrNullID(DL->getScope()));
       Vals.push_back(VE.getMetadataOrNullID(DL->getInlinedAt()));
+      Vals.push_back(DL->isImplicitCode());
       Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_LOC, Vals);
       Vals.clear();
 
       LastDL = DL;
     }
 
   // Emit names for all the instructions etc.
   if (auto *Symtab = F.getValueSymbolTable())

llvm/trunk/lib/IR/AsmWriter.cpp

static void writeDILocation(raw_ostream &Out, const DILocation *DL,
                             const Module *Context) {
   Out << "!DILocation(";
   MDFieldPrinter Printer(Out, TypePrinter, Machine, Context);
   // Always output the line, since 0 is a relevant and important value for it.
   Printer.printInt("line", DL->getLine(), /* ShouldSkipZero */ false);
   Printer.printInt("column", DL->getColumn());
   Printer.printMetadata("scope", DL->getRawScope(), /* ShouldSkipNull */ false);
   Printer.printMetadata("inlinedAt", DL->getRawInlinedAt());
+  Printer.printBool("isImplicitCode", DL->isImplicitCode(),
+                    /* Default */ false);
   Out << ")";
 }
 
 static void writeDISubrange(raw_ostream &Out, const DISubrange *N,
                             TypePrinting *TypePrinter, SlotTracker *Machine,
                             const Module *Context) {
   Out << "!DISubrange(";
   MDFieldPrinter Printer(Out, TypePrinter, Machine, Context);

llvm/trunk/lib/IR/DebugInfoMetadata.cpp

 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instructions.h"
 
 using namespace llvm;
 
 DILocation::DILocation(LLVMContext &C, StorageType Storage, unsigned Line,
-                       unsigned Column, ArrayRef<Metadata *> MDs)
+                       unsigned Column, ArrayRef<Metadata *> MDs,
+                       bool ImplicitCode)
     : MDNode(C, DILocationKind, Storage, MDs) {
   assert((MDs.size() == 1 || MDs.size() == 2) &&
          "Expected a scope and optional inlined-at");
 
   // Set line and column.
   assert(Column < (1u << 16) && "Expected 16-bit column");
 
   SubclassData32 = Line;
   SubclassData16 = Column;
+
+  setImplicitCode(ImplicitCode);
 }
 
 static void adjustColumn(unsigned &Column) {
   // Set to unknown on overflow.  We only have 16 bits to play with here.
   if (Column >= (1u << 16))
     Column = 0;
 }
 
 DILocation *DILocation::getImpl(LLVMContext &Context, unsigned Line,
                                 unsigned Column, Metadata *Scope,
-                                Metadata *InlinedAt, StorageType Storage,
-                                bool ShouldCreate) {
+                                Metadata *InlinedAt, bool ImplicitCode,
+                                StorageType Storage, bool ShouldCreate) {
   // Fixup column.
   adjustColumn(Column);
 
   if (Storage == Uniqued) {
-    if (auto *N =
-            getUniqued(Context.pImpl->DILocations,
-                       DILocationInfo::KeyTy(Line, Column, Scope, InlinedAt)))
+    if (auto *N = getUniqued(Context.pImpl->DILocations,
+                             DILocationInfo::KeyTy(Line, Column, Scope,
+                                                   InlinedAt, ImplicitCode)))
       return N;
     if (!ShouldCreate)
       return nullptr;
   } else {
     assert(ShouldCreate && "Expected non-uniqued nodes to always be created");
   }
 
   SmallVector<Metadata *, 2> Ops;
   Ops.push_back(Scope);
   if (InlinedAt)
     Ops.push_back(InlinedAt);
-  return storeImpl(new (Ops.size())
-                       DILocation(Context, Storage, Line, Column, Ops),
+  return storeImpl(new (Ops.size()) DILocation(Context, Storage, Line, Column,
+                                               Ops, ImplicitCode),
                    Storage, Context.pImpl->DILocations);
 }
 
 const DILocation *DILocation::getMergedLocation(const DILocation *LocA,
                                                 const DILocation *LocB) {
   if (!LocA || !LocB)
     return nullptr;
 

llvm/trunk/lib/IR/DebugLoc.cpp

DebugLoc DebugLoc::getFnDebugLoc() const {
   // FIXME: Add a method on \a DILocation that does this work.
   const MDNode *Scope = getInlinedAtScope();
   if (auto *SP = getDISubprogram(Scope))
     return DebugLoc::get(SP->getScopeLine(), 0, SP);
 
   return DebugLoc();
 }
 
+bool DebugLoc::isImplicitCode() const {
+  if (DILocation *Loc = get()) {
+    return Loc->isImplicitCode();
+  }
+  return true;
+}
+
+void DebugLoc::setImplicitCode(bool ImplicitCode) {
+  if (DILocation *Loc = get()) {
+    Loc->setImplicitCode(ImplicitCode);
+  }
+}
+
 DebugLoc DebugLoc::get(unsigned Line, unsigned Col, const MDNode *Scope,
-                       const MDNode *InlinedAt) {
+                       const MDNode *InlinedAt, bool ImplicitCode) {
   // If no scope is available, this is an unknown location.
   if (!Scope)
     return DebugLoc();
 
   return DILocation::get(Scope->getContext(), Line, Col,
                          const_cast<MDNode *>(Scope),
-                         const_cast<MDNode *>(InlinedAt));
+                         const_cast<MDNode *>(InlinedAt), ImplicitCode);
 }
 
 DebugLoc DebugLoc::appendInlinedAt(DebugLoc DL, DILocation *InlinedAt,
                                    LLVMContext &Ctx,
                                    DenseMap<const MDNode *, MDNode *> &Cache,
                                    bool ReplaceLast) {
   SmallVector<DILocation *, 3> InlinedAtLocations;
   DILocation *Last = InlinedAt;

llvm/trunk/lib/IR/LLVMContextImpl.h

 };
 
 /// DenseMapInfo for DILocation.
 template <> struct MDNodeKeyImpl<DILocation> {
   unsigned Line;
   unsigned Column;
   Metadata *Scope;
   Metadata *InlinedAt;
+  bool ImplicitCode;
 
   MDNodeKeyImpl(unsigned Line, unsigned Column, Metadata *Scope,
-                Metadata *InlinedAt)
-      : Line(Line), Column(Column), Scope(Scope), InlinedAt(InlinedAt) {}
+                Metadata *InlinedAt, bool ImplicitCode)
+      : Line(Line), Column(Column), Scope(Scope), InlinedAt(InlinedAt),
+        ImplicitCode(ImplicitCode) {}
   MDNodeKeyImpl(const DILocation *L)
       : Line(L->getLine()), Column(L->getColumn()), Scope(L->getRawScope()),
-        InlinedAt(L->getRawInlinedAt()) {}
+        InlinedAt(L->getRawInlinedAt()), ImplicitCode(L->isImplicitCode()) {}
 
   bool isKeyOf(const DILocation *RHS) const {
     return Line == RHS->getLine() && Column == RHS->getColumn() &&
-           Scope == RHS->getRawScope() && InlinedAt == RHS->getRawInlinedAt();
+           Scope == RHS->getRawScope() && InlinedAt == RHS->getRawInlinedAt() &&
+           ImplicitCode == RHS->isImplicitCode();
   }
 
   unsigned getHashValue() const {
-    return hash_combine(Line, Column, Scope, InlinedAt);
+    return hash_combine(Line, Column, Scope, InlinedAt, ImplicitCode);
   }
 };
 
 /// DenseMapInfo for GenericDINode.
 template <> struct MDNodeKeyImpl<GenericDINode> : MDNodeOpsKey {
   unsigned Tag;
   MDString *Header;
 

llvm/trunk/lib/Transforms/Instrumentation/GCOVProfiling.cpp

for (auto &F : M->functions()) {
           // declaration, not necessarily any statements or expressions.
           if (isa<DbgInfoIntrinsic>(&I)) continue;
 
           const DebugLoc &Loc = I.getDebugLoc();
           if (!Loc)
             continue;
 
           // Artificial lines such as calls to the global constructors.
-          if (Loc.getLine() == 0) continue;
+          if (Loc.getLine() == 0 || Loc.isImplicitCode())
+            continue;
 
           if (Line == Loc.getLine()) continue;
           Line = Loc.getLine();
           if (SP != getDISubprogram(Loc.getScope()))
             continue;
 
           GCOVLines &Lines = Block.getFile(SP->getFilename());
           Lines.addLine(Loc.getLine());

llvm/trunk/test/Assembler/dilocation.ll

 ; RUN: llvm-as < %s | llvm-dis | FileCheck %s
 ; RUN: verify-uselistorder %s
 
-; CHECK: !named = !{!0, !2, !3, !3, !4, !4, !5, !5, !6}
-!named = !{!0, !2, !3, !4, !5, !6, !7, !8, !9}
+; CHECK: !named = !{!0, !2, !3, !3, !4, !4, !5, !5, !6, !7, !8}
+!named = !{!0, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11}
 
-!llvm.module.flags = !{!10}
+!llvm.module.flags = !{!12}
 !llvm.dbg.cu = !{!1}
 
 ; CHECK: !0 = distinct !DISubprogram(
 !0 = distinct !DISubprogram(unit: !1)
 ; CHECK: !1 = distinct !DICompileUnit
 !1 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "clang",
                              file: !2,
                              isOptimized: true, flags: "-O2",

 
 ; CHECK-NEXT: !5 = !DILocation(line: 0, scope: !0)
 !7 = !DILocation(scope: !0)
 !8 = !DILocation(scope: !0, column: 0, line: 0)
 
 ; CHECK-NEXT: !6 = !DILocation(line: 4294967295, column: 65535, scope: !0)
 !9 = !DILocation(line: 4294967295, column: 65535, scope: !0)
 
-!10 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !DILocation(scope: !0, column: 0, line: 0, isImplicitCode: true)
+!11 = !DILocation(scope: !0, column: 0, line: 1, isImplicitCode: false)
+; CHECK-NEXT: !7 = !DILocation(line: 0, scope: !0, isImplicitCode: true)
+; CHECK-NEXT: !8 = !DILocation(line: 1, scope: !0)
+
+!12 = !{i32 2, !"Debug Info Version", i32 3}

llvm/trunk/test/Bitcode/DILocation-implicit-code.ll

+; RUN: llvm-dis %s.bc -o - | FileCheck %s
+; ModuleID = 'DILocation-implicit-code.cpp'
+source_filename = "DILocation-implicit-code.cpp"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.A = type { i8 }
+
+$_ZN1AC2Ev = comdat any
+
+$_ZN1A3fooEi = comdat any
+
+@_ZTIi = external dso_local constant i8*
+
+; Function Attrs: noinline optnone uwtable
+define dso_local void @_Z5test1v() #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) !dbg !7 {
+entry:
+  %retval = alloca %struct.A, align 1
+  %a = alloca %struct.A, align 1
+  %exn.slot = alloca i8*
+  %ehselector.slot = alloca i32
+  %undef.agg.tmp = alloca %struct.A, align 1
+  %e = alloca i32, align 4
+  %undef.agg.tmp3 = alloca %struct.A, align 1
+  call void @_ZN1AC2Ev(%struct.A* %a), !dbg !9
+  invoke void @_ZN1A3fooEi(%struct.A* %a, i32 0)
+          to label %invoke.cont unwind label %lpad, !dbg !10
+
+invoke.cont:                                      ; preds = %entry
+  br label %return, !dbg !11
+
+lpad:                                             ; preds = %entry
+  %0 = landingpad { i8*, i32 }
+          catch i8* bitcast (i8** @_ZTIi to i8*), !dbg !12
+  %1 = extractvalue { i8*, i32 } %0, 0, !dbg !12
+  store i8* %1, i8** %exn.slot, align 8, !dbg !12
+  %2 = extractvalue { i8*, i32 } %0, 1, !dbg !12
+  store i32 %2, i32* %ehselector.slot, align 4, !dbg !12
+  br label %catch.dispatch, !dbg !12
+
+catch.dispatch:                                   ; preds = %lpad
+  %sel = load i32, i32* %ehselector.slot, align 4, !dbg !13
+  %3 = call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTIi to i8*)) #4, !dbg !13
+  %matches = icmp eq i32 %sel, %3, !dbg !13
+  br i1 %matches, label %catch, label %eh.resume, !dbg !13
+
+catch:                                            ; preds = %catch.dispatch
+  %exn = load i8*, i8** %exn.slot, align 8, !dbg !13
+  %4 = call i8* @__cxa_begin_catch(i8* %exn) #4, !dbg !13
+  %5 = bitcast i8* %4 to i32*, !dbg !13
+  %6 = load i32, i32* %5, align 4, !dbg !13
+  store i32 %6, i32* %e, align 4, !dbg !13
+  %7 = load i32, i32* %e, align 4, !dbg !14
+  invoke void @_ZN1A3fooEi(%struct.A* %a, i32 %7)
+          to label %invoke.cont2 unwind label %lpad1, !dbg !15
+
+invoke.cont2:                                     ; preds = %catch
+  call void @__cxa_end_catch() #4, !dbg !16
+  br label %return
+
+lpad1:                                            ; preds = %catch
+  %8 = landingpad { i8*, i32 }
+          cleanup, !dbg !12
+  %9 = extractvalue { i8*, i32 } %8, 0, !dbg !12
+  store i8* %9, i8** %exn.slot, align 8, !dbg !12
+  %10 = extractvalue { i8*, i32 } %8, 1, !dbg !12
+  store i32 %10, i32* %ehselector.slot, align 4, !dbg !12
+  call void @__cxa_end_catch() #4, !dbg !16
+  br label %eh.resume, !dbg !16
+
+try.cont:                                         ; No predecessors!
+  call void @llvm.trap(), !dbg !16
+  unreachable, !dbg !16
+
+return:                                           ; preds = %invoke.cont2, %invoke.cont
+  ret void, !dbg !12
+
+eh.resume:                                        ; preds = %lpad1, %catch.dispatch
+  %exn4 = load i8*, i8** %exn.slot, align 8, !dbg !13
+  %sel5 = load i32, i32* %ehselector.slot, align 4, !dbg !13
+  %lpad.val = insertvalue { i8*, i32 } undef, i8* %exn4, 0, !dbg !13
+  %lpad.val6 = insertvalue { i8*, i32 } %lpad.val, i32 %sel5, 1, !dbg !13
+  resume { i8*, i32 } %lpad.val6, !dbg !13
+}
+
+; Function Attrs: noinline nounwind optnone uwtable
+define linkonce_odr dso_local void @_ZN1AC2Ev(%struct.A* %this) unnamed_addr #1 comdat align 2 !dbg !17 {
+entry:
+  %this.addr = alloca %struct.A*, align 8
+  store %struct.A* %this, %struct.A** %this.addr, align 8
+  %this1 = load %struct.A*, %struct.A** %this.addr, align 8
+  ret void, !dbg !18
+}
+
+; Function Attrs: noinline optnone uwtable
+define linkonce_odr dso_local void @_ZN1A3fooEi(%struct.A* %this, i32 %i) #0 comdat align 2 !dbg !19 {
+entry:
+  %retval = alloca %struct.A, align 1
+  %this.addr = alloca %struct.A*, align 8
+  %i.addr = alloca i32, align 4
+  store %struct.A* %this, %struct.A** %this.addr, align 8
+  store i32 %i, i32* %i.addr, align 4
+  %this1 = load %struct.A*, %struct.A** %this.addr, align 8
+  %0 = load i32, i32* %i.addr, align 4, !dbg !20
+  %cmp = icmp eq i32 %0, 0, !dbg !21
+  br i1 %cmp, label %if.then, label %if.end, !dbg !20
+
+if.then:                                          ; preds = %entry
+  %exception = call i8* @__cxa_allocate_exception(i64 4) #4, !dbg !22
+  %1 = bitcast i8* %exception to i32*, !dbg !22
+  store i32 1, i32* %1, align 16, !dbg !22
+  call void @__cxa_throw(i8* %exception, i8* bitcast (i8** @_ZTIi to i8*), i8* null) #5, !dbg !22
+  unreachable, !dbg !22
+
+if.end:                                           ; preds = %entry
+  ret void, !dbg !23
+}
+
+declare dso_local i32 @__gxx_personality_v0(...)
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.eh.typeid.for(i8*) #2
+
+declare dso_local i8* @__cxa_begin_catch(i8*)
+
+declare dso_local void @__cxa_end_catch()
+
+; Function Attrs: noreturn nounwind
+declare void @llvm.trap() #3
+
+; Function Attrs: noinline optnone uwtable
+define dso_local void @_Z5test2v() #0 !dbg !24 {
+entry:
+  %a = alloca %struct.A, align 1
+  %b = alloca %struct.A, align 1
+  %undef.agg.tmp = alloca %struct.A, align 1
+  call void @_ZN1AC2Ev(%struct.A* %a), !dbg !25
+  call void @_ZN1A3fooEi(%struct.A* %a, i32 1), !dbg !26
+  ret void, !dbg !27
+}
+
+declare dso_local i8* @__cxa_allocate_exception(i64)
+
+declare dso_local void @__cxa_throw(i8*, i8*, i8*)
+
+attributes #0 = { noinline optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #2 = { nounwind readnone }
+attributes #3 = { noreturn nounwind }
+attributes #4 = { nounwind }
+attributes #5 = { noreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+; CHECK: !DILocation(line: 20, column: 1, scope: !{{[0-9]+}}, isImplicitCode: true)
+; CHECK: !DILocation(line: 17, column: 5, scope: !{{[0-9]+}}, isImplicitCode: true)
+; CHECK: !DILocation(line: 19, column: 5, scope: !{{[0-9]+}}, isImplicitCode: true)
+; CHECK: !DILocation(line: 3, column: 10, scope: !{{[0-9]+}}, isImplicitCode: true)
+; CHECK: !DILocation(line: 25, column: 1, scope: !{{[0-9]+}}, isImplicitCode: true)
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 8.0.0 (trunk 342445)", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "DILocation-implicit-code.cpp", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 8.0.0 (trunk 342445)"}
+!7 = distinct !DISubprogram(name: "test1", scope: !1, file: !1, line: 13, type: !8, isLocal: false, isDefinition: true, scopeLine: 13, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+!8 = !DISubroutineType(types: !2)
+!9 = !DILocation(line: 14, column: 7, scope: !7)
+!10 = !DILocation(line: 16, column: 18, scope: !7)
+!11 = !DILocation(line: 16, column: 9, scope: !7)
+!12 = !DILocation(line: 20, column: 1, scope: !7, isImplicitCode: true)
+!13 = !DILocation(line: 17, column: 5, scope: !7, isImplicitCode: true)
+!14 = !DILocation(line: 18, column: 22, scope: !7)
+!15 = !DILocation(line: 18, column: 18, scope: !7)
+!16 = !DILocation(line: 19, column: 5, scope: !7, isImplicitCode: true)
+!17 = distinct !DISubprogram(name: "A", scope: !1, file: !1, line: 3, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+!18 = !DILocation(line: 3, column: 10, scope: !17, isImplicitCode: true)
+!19 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, type: !8, isLocal: false, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+!20 = !DILocation(line: 5, column: 13, scope: !19)
+!21 = !DILocation(line: 5, column: 15, scope: !19)
+!22 = !DILocation(line: 6, column: 13, scope: !19)
+!23 = !DILocation(line: 9, column: 9, scope: !19)
+!24 = distinct !DISubprogram(name: "test2", scope: !1, file: !1, line: 22, type: !8, isLocal: false, isDefinition: true, scopeLine: 22, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+!25 = !DILocation(line: 23, column: 7, scope: !24)
+!26 = !DILocation(line: 24, column: 13, scope: !24)
+!27 = !DILocation(line: 25, column: 1, scope: !24, isImplicitCode: true)