probinson created this revision.
probinson added reviewers: dblaikie, rnk.
probinson added a project: debug-info.
probinson requested review of this revision.
Herald added a project: clang.

Attaches a more appropriate debug location to the PHIs used for the
short-circuit evaluations, and makes logical-and and logical-or
handling consistent.  Also sets the appropriate debug location for all
scalar conversions, which incidentally does the right thing for
int-to-bool conversions in particular.

While this does tidy up the source locations emitted to IR, it doesn't
appear to make any difference in the final object file for simple
tests.  However, it should permit re-applying D91734 
<https://reviews.llvm.org/D91734> and friends to
improve the debug info without exposing the regressions that prompted
the reverts in #615f63e.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92606

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/debug-info-inline-for.c
  clang/test/CodeGen/debug-info-land-lor.c

Index: clang/test/CodeGen/debug-info-land-lor.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/debug-info-land-lor.c
@@ -0,0 +1,36 @@
+// Check that clang emits reasonable debug info for && and ||.
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+int f1(int a, int b) {
+  return a && b; // line 4
+}
+int f2(int a, int b) {
+  return a || b; // line 7
+}
+
+// Each load/icmp pair should have the location of the operand.
+// Each conditional branch/phi pair should have the location of the operator.
+
+// CHECK-LABEL: @f1
+// CHECK:      load {{.*}} %a.addr, {{.*}} !dbg ![[F1A:[0-9]+]]
+// CHECK-NEXT: icmp {{.*}} !dbg ![[F1A]]
+// CHECK-NEXT: br {{.*}} !dbg ![[F1AND:[0-9]+]]
+// CHECK: land.rhs:
+// CHECK-NEXT: load {{.*}} %b.addr, {{.*}} !dbg ![[F1B:[0-9]+]]
+// CHECK-NEXT: icmp {{.*}} !dbg ![[F1B]]
+// CHECK:      phi {{.*}} !dbg ![[F1AND]]
+
+// CHECK-LABEL: @f2
+// CHECK:      load {{.*}} %a.addr, {{.*}} !dbg ![[F2A:[0-9]+]]
+// CHECK-NEXT: icmp {{.*}} !dbg ![[F2A]]
+// CHECK-NEXT: br {{.*}} !dbg ![[F2OR:[0-9]+]]
+// CHECK: lor.rhs:
+// CHECK-NEXT: load {{.*}} %b.addr, {{.*}} !dbg ![[F2B:[0-9]+]]
+// CHECK-NEXT: icmp {{.*}} !dbg ![[F2B]]
+// CHECK:      phi {{.*}} !dbg ![[F2OR]]
+
+// CHECK-DAG:   ![[F1A]] = !DILocation(line: 4, column: 10,
+// CHECK-DAG:   ![[F1B]] = !DILocation(line: 4, column: 15,
+// CHECK-DAG: ![[F1AND]] = !DILocation(line: 4, column: 12,
+// CHECK-DAG:   ![[F2A]] = !DILocation(line: 7, column: 10,
+// CHECK-DAG:   ![[F2B]] = !DILocation(line: 7, column: 15,
+// CHECK-DAG:  ![[F2OR]] = !DILocation(line: 7, column: 12,
Index: clang/test/CodeGen/debug-info-inline-for.c
===================================================================
--- clang/test/CodeGen/debug-info-inline-for.c
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
-// Check that clang emits Debug location in the phi instruction
-
-int func(int n) {
-  int a;
-  for(a = 10; a>0 && n++; a--);
-  return n;
-}
-
-// CHECK: land.end:
-// CHECK-NEXT: {{.*}} = phi i1 {{.*}} !dbg ![[DbgLoc:[0-9]+]]
-
-// CHECK: ![[DbgLoc]] = !DILocation(line: 0
Index: clang/lib/CodeGen/CGExprScalar.cpp
===================================================================
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1197,6 +1197,8 @@
                                                QualType DstType,
                                                SourceLocation Loc,
                                                ScalarConversionOpts Opts) {
+  ApplyDebugLocation DL(CGF, Loc);
+
   // All conversions involving fixed point types should be handled by the
   // EmitFixedPoint family functions. This is done to prevent bloating up this
   // function more, and although fixed point numbers are represented by
@@ -4187,6 +4189,7 @@
   // setting up the PHI node in the Cont Block for this.
   llvm::PHINode *PN = llvm::PHINode::Create(llvm::Type::getInt1Ty(VMContext), 2,
                                             "", ContBlock);
+  PN->setDebugLoc(Builder.getCurrentDebugLocation());
   for (llvm::pred_iterator PI = pred_begin(ContBlock), PE = pred_end(ContBlock);
        PI != PE; ++PI)
     PN->addIncoming(llvm::ConstantInt::getFalse(VMContext), *PI);
@@ -4209,12 +4212,6 @@
   // Insert an entry into the phi node for the edge with the value of RHSCond.
   PN->addIncoming(RHSCond, RHSBlock);
 
-  // Artificial location to preserve the scope information
-  {
-    auto NL = ApplyDebugLocation::CreateArtificial(CGF);
-    PN->setDebugLoc(Builder.getCurrentDebugLocation());
-  }
-
   // ZExt result to int.
   return Builder.CreateZExtOrBitCast(PN, ResTy, "land.ext");
 }
@@ -4274,6 +4271,7 @@
   // setting up the PHI node in the Cont Block for this.
   llvm::PHINode *PN = llvm::PHINode::Create(llvm::Type::getInt1Ty(VMContext), 2,
                                             "", ContBlock);
+  PN->setDebugLoc(Builder.getCurrentDebugLocation());
   for (llvm::pred_iterator PI = pred_begin(ContBlock), PE = pred_end(ContBlock);
        PI != PE; ++PI)
     PN->addIncoming(llvm::ConstantInt::getTrue(VMContext), *PI);
@@ -4290,9 +4288,13 @@
   // Reaquire the RHS block, as there may be subblocks inserted.
   RHSBlock = Builder.GetInsertBlock();
 
-  // Emit an unconditional branch from this block to ContBlock.  Insert an entry
-  // into the phi node for the edge with the value of RHSCond.
-  CGF.EmitBlock(ContBlock);
+  // Emit an unconditional branch from this block to ContBlock.
+  {
+    // There is no need to emit line number for unconditional branch.
+    auto NL = ApplyDebugLocation::CreateEmpty(CGF);
+    CGF.EmitBlock(ContBlock);
+  }
+  // Insert an entry into the phi node for the edge with the value of RHSCond.
   PN->addIncoming(RHSCond, RHSBlock);
 
   // ZExt result to int.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to