Debug info: Fixed faulty debug locations for attributed statements

[Ka-Ka]

Summary

As the attributed statements are considered simple statements no
stoppoint was generated before emitting attributed do/while/for/range-
statement. This lead to faulty debug locations.

[Ka-Ka]

test/CodeGen/debug-info-attributed-stmt.c

18 ↗

(On Diff #113708)

The part "scope: !14)" should be removed from the testcase.

[Ka-Ka]

I updated the testcase according to what David Blaikie suggested.
I also rewrote to code to be a bit more robust and avoid emitting unnecessary stoppoints.

[dblaikie]

Not sure what you mean by "avoid emitting unnecessary stop points" - do you have a test case for that?

[Ka-Ka]

In D37428#861140, @dblaikie wrote:

Not sure what you mean by "avoid emitting unnecessary stop points" - do you have a test case for that?

In my previous patch you could end up doing two calls to EmitStopPoint() for the same statement. In the previous patch I added a EmitStopPoint() in the method EmitAttributedStmt(), but if you took the default clause in the switch statement in the same method you could end up executing another EmitStopPoint() in EmitStmt().

The new patch do not have the issue with executing EmitStopPoint() twice for the same statement. I think generally that the new patch is more robust solution.

[dblaikie]

OK, sounds good - thanks :)

[echristo]

test/CodeGen/debug-info-attributed-stmt.c

1 ↗

(On Diff #113803)

Since we're not optimizing or generating code you should be able to remove the -disable-llvm-passes I believe.

[Ka-Ka]

test/CodeGen/debug-info-attributed-stmt.c

1 ↗

(On Diff #113803)

I added the option -disable-llvm-passes as David Blaikie suggested it in his original comment:
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20170904/202982.html
As I have mainly worked in llvm backends I'm not that familiar with the clang -cc1 options. I have no opinion about this. I wait for you and David Blaikie to decide if I should remove or keep the option -disable-llvm-passes.

cfe/trunk/lib/CodeGen/CGStmt.cpp

if (CGDebugInfo *DI = getDebugInfo()) {
     SourceLocation Loc;
     Loc = S->getLocStart();
     DI->EmitLocation(Builder, Loc);
 
     LastStopPoint = Loc;
   }
 }
 
-void CodeGenFunction::EmitStmt(const Stmt *S) {
+void CodeGenFunction::EmitStmt(const Stmt *S, ArrayRef<const Attr *> Attrs) {
   assert(S && "Null statement?");
   PGO.setCurrentStmt(S);
 
   // These statements have their own debug info handling.
   if (EmitSimpleStmt(S))
     return;
 
   // Check if we are generating unreachable code.
if (incoming != outgoing && outgoing->use_empty()) {
       Builder.ClearInsertionPoint();
     }
     break;
   }
 
   case Stmt::IndirectGotoStmtClass:
     EmitIndirectGotoStmt(cast<IndirectGotoStmt>(*S)); break;
 
-  case Stmt::IfStmtClass:       EmitIfStmt(cast<IfStmt>(*S));             break;
-  case Stmt::WhileStmtClass:    EmitWhileStmt(cast<WhileStmt>(*S));       break;
-  case Stmt::DoStmtClass:       EmitDoStmt(cast<DoStmt>(*S));             break;
-  case Stmt::ForStmtClass:      EmitForStmt(cast<ForStmt>(*S));           break;
+  case Stmt::IfStmtClass:      EmitIfStmt(cast<IfStmt>(*S));              break;
+  case Stmt::WhileStmtClass:   EmitWhileStmt(cast<WhileStmt>(*S), Attrs); break;
+  case Stmt::DoStmtClass:      EmitDoStmt(cast<DoStmt>(*S), Attrs);       break;
+  case Stmt::ForStmtClass:     EmitForStmt(cast<ForStmt>(*S), Attrs);     break;
 
-  case Stmt::ReturnStmtClass:   EmitReturnStmt(cast<ReturnStmt>(*S));     break;
+  case Stmt::ReturnStmtClass:  EmitReturnStmt(cast<ReturnStmt>(*S));      break;
 
-  case Stmt::SwitchStmtClass:   EmitSwitchStmt(cast<SwitchStmt>(*S));     break;
-  case Stmt::GCCAsmStmtClass:   // Intentional fall-through.
-  case Stmt::MSAsmStmtClass:    EmitAsmStmt(cast<AsmStmt>(*S));           break;
+  case Stmt::SwitchStmtClass:  EmitSwitchStmt(cast<SwitchStmt>(*S));      break;
+  case Stmt::GCCAsmStmtClass:  // Intentional fall-through.
+  case Stmt::MSAsmStmtClass:   EmitAsmStmt(cast<AsmStmt>(*S));            break;
   case Stmt::CoroutineBodyStmtClass:
     EmitCoroutineBody(cast<CoroutineBodyStmt>(*S));
     break;
   case Stmt::CoreturnStmtClass:
     EmitCoreturnStmt(cast<CoreturnStmt>(*S));
     break;
   case Stmt::CapturedStmtClass: {
     const CapturedStmt *CS = cast<CapturedStmt>(S);
#include "clang/AST/StmtNodes.inc"
   case Stmt::ObjCAutoreleasePoolStmtClass:
     EmitObjCAutoreleasePoolStmt(cast<ObjCAutoreleasePoolStmt>(*S));
     break;
 
   case Stmt::CXXTryStmtClass:
     EmitCXXTryStmt(cast<CXXTryStmt>(*S));
     break;
   case Stmt::CXXForRangeStmtClass:
-    EmitCXXForRangeStmt(cast<CXXForRangeStmt>(*S));
+    EmitCXXForRangeStmt(cast<CXXForRangeStmt>(*S), Attrs);
     break;
   case Stmt::SEHTryStmtClass:
     EmitSEHTryStmt(cast<SEHTryStmt>(*S));
     break;
   case Stmt::OMPParallelDirectiveClass:
     EmitOMPParallelDirective(cast<OMPParallelDirective>(*S));
     break;
   case Stmt::OMPSimdDirectiveClass:

 
 
 void CodeGenFunction::EmitLabelStmt(const LabelStmt &S) {
   EmitLabel(S.getDecl());
   EmitStmt(S.getSubStmt());
 }
 
 void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) {
-  const Stmt *SubStmt = S.getSubStmt();
-  switch (SubStmt->getStmtClass()) {
-  case Stmt::DoStmtClass:
-    EmitDoStmt(cast<DoStmt>(*SubStmt), S.getAttrs());
-    break;
-  case Stmt::ForStmtClass:
-    EmitForStmt(cast<ForStmt>(*SubStmt), S.getAttrs());
-    break;
-  case Stmt::WhileStmtClass:
-    EmitWhileStmt(cast<WhileStmt>(*SubStmt), S.getAttrs());
-    break;
-  case Stmt::CXXForRangeStmtClass:
-    EmitCXXForRangeStmt(cast<CXXForRangeStmt>(*SubStmt), S.getAttrs());
-    break;
-  default:
-    EmitStmt(SubStmt);
-  }
+  EmitStmt(S.getSubStmt(), S.getAttrs());
 }
 
 void CodeGenFunction::EmitGotoStmt(const GotoStmt &S) {
   // If this code is reachable then emit a stop point (if generating
   // debug info). We have to do this ourselves because we are on the
   // "simple" statement path.
   if (HaveInsertPoint())
     EmitStopPoint(&S);

cfe/trunk/lib/CodeGen/CodeGenFunction.h

#endif
   void EmitStopPoint(const Stmt *S);
 
   /// EmitStmt - Emit the code for the statement \arg S. It is legal to call
   /// this function even if there is no current insertion point.
   ///
   /// This function may clear the current insertion point; callers should use
   /// EnsureInsertPoint if they wish to subsequently generate code without first
   /// calling EmitBlock, EmitBranch, or EmitStmt.
-  void EmitStmt(const Stmt *S);
+  void EmitStmt(const Stmt *S, ArrayRef<const Attr *> Attrs = None);
 
   /// EmitSimpleStmt - Try to emit a "simple" statement which does not
   /// necessarily require an insertion point or debug information; typically
   /// because the statement amounts to a jump or a container of other
   /// statements.
   ///
   /// \return True if the statement was handled.
   bool EmitSimpleStmt(const Stmt *S);

cfe/trunk/test/CodeGen/debug-info-attributed-stmt.c

+// RUN: %clang_cc1 -triple x86_64-unk-unk -debug-info-kind=limited -emit-llvm %s -o - | FileCheck %s
+
+void f(_Bool b)
+{
+#pragma nounroll
+  while (b);
+}
+
+// CHECK: br label %while.cond, !dbg ![[NUM:[0-9]+]]
+// CHECK: br i1 %tobool, label %while.body, label %while.end, !dbg ![[NUM]]
+// CHECK: br label %while.cond, !dbg ![[NUM]], !llvm.loop
+// CHECK: ![[NUM]] = !DILocation(line: 6,