Add a loop's debug location to its llvm.loop metadata

Getting accurate locations for loops is important, because those locations are
used by the frontend to generate optimization remarks. Currently, optimization
remarks for loops often appear on the wrong line, often the first line of the
loop body instead of the loop itself. This is confusing because that line might
itself be another loop, or might be somewhere else completely if the body was
an inlined function call. This happens because of the way we find the loop's
starting location. First, we look for a preheader, and if we find one, and its
terminator has a debug location, then we use that. Otherwise, we look for a
location on an instruction in the loop header.

The fallback heuristic is not bad, but will almost always find the beginning of
the body, and not the loop statement itself. The preheader location search
often fails because there's often not a preheader, and even when there is a
preheader, depending on how it was formed, it sometimes carries the location of
some preceeding code.

I don't see any good theoretical way to fix this problem. On the other hand,
this seems like a straightforward solution: Put the debug location in the
loop's llvm.loop metadata. When emitting debug information, this commit causes
us to add the debug location as an operand to each loop's llvm.loop metadata.
Thus, we now generate this metadata for all loops (not just loops with
optimization hints) when we're otherwise generating debug information.

The remark test case changes depend on the companion LLVM commit r270771.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@270772 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/CGLoopInfo.cpp b/lib/CodeGen/CGLoopInfo.cpp
index 08ed173f33e296518ae5d0aeae78625111b18a41..e07fbb9bb3dca1cd3afa43e0ef09a59714966977 100644 (file)
--- a/lib/CodeGen/CGLoopInfo.cpp
+++ b/lib/CodeGen/CGLoopInfo.cpp
@@ -19,12 +19,14 @@
 using namespace clang::CodeGen;
 using namespace llvm;
 
-static MDNode *createMetadata(LLVMContext &Ctx, const LoopAttributes &Attrs) {
+static MDNode *createMetadata(LLVMContext &Ctx, const LoopAttributes &Attrs,
+                              llvm::DebugLoc Location) {
 
   if (!Attrs.IsParallel && Attrs.VectorizeWidth == 0 &&
       Attrs.InterleaveCount == 0 && Attrs.UnrollCount == 0 &&
       Attrs.VectorizeEnable == LoopAttributes::Unspecified &&
-      Attrs.UnrollEnable == LoopAttributes::Unspecified)
+      Attrs.UnrollEnable == LoopAttributes::Unspecified &&
+      !Location)
     return nullptr;
 
   SmallVector<Metadata *, 4> Args;
@@ -32,6 +34,10 @@ static MDNode *createMetadata(LLVMContext &Ctx, const LoopAttributes &Attrs) {
   auto TempNode = MDNode::getTemporary(Ctx, None);
   Args.push_back(TempNode.get());
 
+  // If we have a valid debug location for the loop, add it.
+  if (Location)
+    Args.push_back(Location.getAsMDNode());
+
   // Setting vectorize.width
   if (Attrs.VectorizeWidth > 0) {
     Metadata *Vals[] = {MDString::get(Ctx, "llvm.loop.vectorize.width"),
@@ -98,19 +104,21 @@ void LoopAttributes::clear() {
   UnrollEnable = LoopAttributes::Unspecified;
 }
 
-LoopInfo::LoopInfo(BasicBlock *Header, const LoopAttributes &Attrs)
+LoopInfo::LoopInfo(BasicBlock *Header, const LoopAttributes &Attrs,
+                   llvm::DebugLoc Location)
     : LoopID(nullptr), Header(Header), Attrs(Attrs) {
-  LoopID = createMetadata(Header->getContext(), Attrs);
+  LoopID = createMetadata(Header->getContext(), Attrs, Location);
 }
 
-void LoopInfoStack::push(BasicBlock *Header) {
-  Active.push_back(LoopInfo(Header, StagedAttrs));
+void LoopInfoStack::push(BasicBlock *Header, llvm::DebugLoc Location) {
+  Active.push_back(LoopInfo(Header, StagedAttrs, Location));
   // Clear the attributes so nested loops do not inherit them.
   StagedAttrs.clear();
 }
 
 void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx,
-                         ArrayRef<const clang::Attr *> Attrs) {
+                         ArrayRef<const clang::Attr *> Attrs,
+                         llvm::DebugLoc Location) {
 
   // Identify loop hint attributes from Attrs.
   for (const auto *Attr : Attrs) {
@@ -239,7 +247,7 @@ void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx,
   }
 
   /// Stage the attributes.
-  push(Header);
+  push(Header, Location);
 }
 
 void LoopInfoStack::pop() {

diff --git a/lib/CodeGen/CGLoopInfo.h b/lib/CodeGen/CGLoopInfo.h
index ec3390677fa95a8376e85794169ead730ae9c4e7..7b9980c61837d2dd330ff494e4646601a7ab4fe4 100644 (file)
--- a/lib/CodeGen/CGLoopInfo.h
+++ b/lib/CodeGen/CGLoopInfo.h
@@ -18,6 +18,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/Compiler.h"
 
@@ -63,7 +64,8 @@ struct LoopAttributes {
 class LoopInfo {
 public:
   /// \brief Construct a new LoopInfo for the loop with entry Header.
-  LoopInfo(llvm::BasicBlock *Header, const LoopAttributes &Attrs);
+  LoopInfo(llvm::BasicBlock *Header, const LoopAttributes &Attrs,
+           llvm::DebugLoc Location);
 
   /// \brief Get the loop id metadata for this loop.
   llvm::MDNode *getLoopID() const { return LoopID; }
@@ -95,12 +97,14 @@ public:
 
   /// \brief Begin a new structured loop. The set of staged attributes will be
   /// applied to the loop and then cleared.
-  void push(llvm::BasicBlock *Header);
+  void push(llvm::BasicBlock *Header,
+            llvm::DebugLoc Location = llvm::DebugLoc());
 
   /// \brief Begin a new structured loop. Stage attributes from the Attrs list.
   /// The staged attributes are applied to the loop and then cleared.
   void push(llvm::BasicBlock *Header, clang::ASTContext &Ctx,
-            llvm::ArrayRef<const Attr *> Attrs);
+            llvm::ArrayRef<const Attr *> Attrs,
+            llvm::DebugLoc Location = llvm::DebugLoc());
 
   /// \brief End the current loop.
   void pop();

diff --git a/lib/CodeGen/CGStmt.cpp b/lib/CodeGen/CGStmt.cpp
index 73a941a36825e14f65e2a56161ce25ff4b319e40..a4f59fbc19db08062cc98af9b28c4dc9e8de1c80 100644 (file)
--- a/lib/CodeGen/CGStmt.cpp
+++ b/lib/CodeGen/CGStmt.cpp
@@ -629,7 +629,8 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
   JumpDest LoopHeader = getJumpDestInCurrentScope("while.cond");
   EmitBlock(LoopHeader.getBlock());
 
-  LoopStack.push(LoopHeader.getBlock(), CGM.getContext(), WhileAttrs);
+  LoopStack.push(LoopHeader.getBlock(), CGM.getContext(), WhileAttrs,
+                 Builder.getCurrentDebugLocation());
 
   // Create an exit block for when the condition fails, which will
   // also become the break target.
@@ -720,7 +721,8 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S,
   // Emit the body of the loop.
   llvm::BasicBlock *LoopBody = createBasicBlock("do.body");
 
-  LoopStack.push(LoopBody, CGM.getContext(), DoAttrs);
+  LoopStack.push(LoopBody, CGM.getContext(), DoAttrs,
+                 Builder.getCurrentDebugLocation());
 
   EmitBlockWithFallThrough(LoopBody, &S);
   {
@@ -772,6 +774,8 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
 
   LexicalScope ForScope(*this, S.getSourceRange());
 
+  llvm::DebugLoc DL = Builder.getCurrentDebugLocation();
+
   // Evaluate the first part before the loop.
   if (S.getInit())
     EmitStmt(S.getInit());
@@ -783,7 +787,7 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
   llvm::BasicBlock *CondBlock = Continue.getBlock();
   EmitBlock(CondBlock);
 
-  LoopStack.push(CondBlock, CGM.getContext(), ForAttrs);
+  LoopStack.push(CondBlock, CGM.getContext(), ForAttrs, DL);
 
   // If the for loop doesn't have an increment we can just use the
   // condition as the continue block.  Otherwise we'll need to create
@@ -868,6 +872,8 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
 
   LexicalScope ForScope(*this, S.getSourceRange());
 
+  llvm::DebugLoc DL = Builder.getCurrentDebugLocation();
+
   // Evaluate the first pieces before the loop.
   EmitStmt(S.getRangeStmt());
   EmitStmt(S.getBeginStmt());
@@ -879,7 +885,7 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
   llvm::BasicBlock *CondBlock = createBasicBlock("for.cond");
   EmitBlock(CondBlock);
 
-  LoopStack.push(CondBlock, CGM.getContext(), ForAttrs);
+  LoopStack.push(CondBlock, CGM.getContext(), ForAttrs, DL);
 
   // If there are any cleanups between here and the loop-exit scope,
   // create a block to stage a loop exit along.

diff --git a/lib/CodeGen/CGStmtOpenMP.cpp b/lib/CodeGen/CGStmtOpenMP.cpp
index 45d8d0af10d7ef62ff463d3f6228406cc9d2da62..b3ff09b143b78b80ad9f373754e6d71238d50b8c 100644 (file)
--- a/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1271,7 +1271,7 @@ void CodeGenFunction::EmitOMPInnerLoop(
   // Start the loop with a block that tests the condition.
   auto CondBlock = createBasicBlock("omp.inner.for.cond");
   EmitBlock(CondBlock);
-  LoopStack.push(CondBlock);
+  LoopStack.push(CondBlock, Builder.getCurrentDebugLocation());
 
   // If there are any cleanups between here and the loop-exit scope,
   // create a block to stage a loop exit along.
@@ -1672,7 +1672,7 @@ void CodeGenFunction::EmitOMPOuterLoop(bool DynamicOrOrdered, bool IsMonotonic,
   // Start the loop with a block that tests the condition.
   auto CondBlock = createBasicBlock("omp.dispatch.cond");
   EmitBlock(CondBlock);
-  LoopStack.push(CondBlock);
+  LoopStack.push(CondBlock, Builder.getCurrentDebugLocation());
 
   llvm::Value *BoolCondVal = nullptr;
   if (!DynamicOrOrdered) {

diff --git a/test/CodeGenCXX/debug-info-line-if.cpp b/test/CodeGenCXX/debug-info-line-if.cpp
index 29806351c94c591e9c81398bdc57c874828c4b91..b3f9c32e09116218c6ba43393669eb9d08c861c9 100644 (file)
--- a/test/CodeGenCXX/debug-info-line-if.cpp
+++ b/test/CodeGenCXX/debug-info-line-if.cpp
@@ -15,7 +15,7 @@ int main() {
 
   // CHECK: br label
   // CHECK: br label
-  // CHECK: br label {{.*}}, !dbg [[DBG1:!.*]]
+  // CHECK: br label {{.*}}, !dbg [[DBG1:![0-9]*]], !llvm.loop [[L1:![0-9]*]]
 
 #line 200
   while (a)
@@ -25,7 +25,7 @@ int main() {
       ++a; // CHECK: add nsw{{.*}}, 1
 
   // CHECK: br label
-  // CHECK: br label {{.*}}, !dbg [[DBG2:!.*]]
+  // CHECK: br label {{.*}}, !dbg [[DBG2:![0-9]*]], !llvm.loop [[L2:![0-9]*]]
 
 #line 300
   for (; a; )
@@ -35,7 +35,7 @@ int main() {
       ++a; // CHECK: add nsw{{.*}}, 1
 
   // CHECK: br label
-  // CHECK: br label {{.*}}, !dbg [[DBG3:!.*]]
+  // CHECK: br label {{.*}}, !dbg [[DBG3:![0-9]*]], !llvm.loop [[L3:![0-9]*]]
 
 #line 400
   int x[] = {1, 2};
@@ -46,10 +46,22 @@ int main() {
       ++a; // CHECK: add nsw{{.*}}, 1
 
   // CHECK: br label
-  // CHECK: br label {{.*}}, !dbg [[DBG4:!.*]]
+  // CHECK: br label {{.*}}, !dbg [[DBG4:![0-9]*]], !llvm.loop [[L4:![0-9]*]]
 
-  // CHECK: [[DBG1]] = !DILocation(line: 100, scope: !{{.*}})
-  // CHECK: [[DBG2]] = !DILocation(line: 200, scope: !{{.*}})
-  // CHECK: [[DBG3]] = !DILocation(line: 300, scope: !{{.*}})
-  // CHECK: [[DBG4]] = !DILocation(line: 401, scope: !{{.*}})
+  // CHECK-DAG: [[DBG1]] = !DILocation(line: 100, scope: !{{.*}})
+  // CHECK-DAG: [[DBG2]] = !DILocation(line: 200, scope: !{{.*}})
+  // CHECK-DAG: [[DBG3]] = !DILocation(line: 300, scope: !{{.*}})
+  // CHECK-DAG: [[DBG4]] = !DILocation(line: 401, scope: !{{.*}})
+
+  // CHECK-DAG: [[L1]] = distinct !{[[L1]], [[LDBG1:![0-9]*]]}
+  // CHECK-DAG: [[LDBG1]] = !DILocation(line: 100, scope: !{{.*}})
+
+  // CHECK-DAG: [[L2]] = distinct !{[[L2]], [[LDBG2:![0-9]*]]}
+  // CHECK-DAG: [[LDBG2]] = !DILocation(line: 200, scope: !{{.*}})
+
+  // CHECK-DAG: [[L3]] = distinct !{[[L3]], [[LDBG3:![0-9]*]]}
+  // CHECK-DAG: [[LDBG3]] = !DILocation(line: 300, scope: !{{.*}})
+
+  // CHECK-DAG: [[L4]] = distinct !{[[L4]], [[LDBG4:![0-9]*]]}
+  // CHECK-DAG: [[LDBG4]] = !DILocation(line: 401, scope: !{{.*}})
 }

diff --git a/test/Frontend/optimization-remark-options.c b/test/Frontend/optimization-remark-options.c
index 74fbeaf5d0c17db43cdbd95daa950eb3de734fd2..a2d717a2422df2d726497c4ce43e2849fac163d0 100644 (file)
--- a/test/Frontend/optimization-remark-options.c
+++ b/test/Frontend/optimization-remark-options.c
@@ -11,7 +11,7 @@ double foo(int N) {
   return v;
 }
 
-// CHECK: {{.*}}:18:13: remark: loop not vectorized: cannot prove it is safe to reorder memory operations; allow reordering by specifying '#pragma clang loop vectorize(enable)' before the loop. If the arrays will always be independent specify '#pragma clang loop vectorize(assume_safety)' before the loop or provide the '__restrict__' qualifier with the independent array arguments. Erroneous results will occur if these options are incorrectly applied!
+// CHECK: {{.*}}:17:3: remark: loop not vectorized: cannot prove it is safe to reorder memory operations; allow reordering by specifying '#pragma clang loop vectorize(enable)' before the loop. If the arrays will always be independent specify '#pragma clang loop vectorize(assume_safety)' before the loop or provide the '__restrict__' qualifier with the independent array arguments. Erroneous results will occur if these options are incorrectly applied!
 
 void foo2(int *dw, int *uw, int *A, int *B, int *C, int *D, int N) {
   for (int i = 0; i < N; i++) {

diff --git a/test/Misc/backend-optimization-failure.cpp b/test/Misc/backend-optimization-failure.cpp
index c0f3bf46f081b15074999f9464094b7a2911dd11..bb50e96aaa57832bfb23eba6d0627a7e739a2adb 100644 (file)
--- a/test/Misc/backend-optimization-failure.cpp
+++ b/test/Misc/backend-optimization-failure.cpp
@@ -7,7 +7,7 @@
 void test_switch(int *A, int *B, int Length) {
 #pragma clang loop vectorize(enable) unroll(disable)
   for (int i = 0; i < Length; i++) {
-/* expected-warning {{loop not vectorized: failed explicitly specified loop vectorization}} */ switch (A[i]) {
+/* expected-warning@-1 {{loop not vectorized: failed explicitly specified loop vectorization}} */ switch (A[i]) {
     case 0:
       B[i] = 1;
       break;