[clang][PGO] Apply artificial DebugLoc to llvm.instrprof.increment instructions

[Michael137]

Prior to this change the debug-location for the llvm.instrprof.increment intrinsic was set to whatever the current DIBuilder's current debug location was set to. This meant that for switch-statements, a counter's location was set to the previous case's debug-location, causing confusing stepping behaviour in debuggers. This patch makes sure we attach a dummy debug-location for the increment instructions.

rdar://123050737

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Michael Buch (Michael137)

Changes

Prior to this change the debug-location for the llvm.instrprof.increment intrinsic was set to whatever the current DIBuilder's current debug location was set to. This meant that for switch-statements, a counter's location was set to the previous case's debug-location, causing confusing stepping behaviour in debuggers. This patch makes sure we attach a dummy debug-location for the increment instructions.

rdar://123050737

---

Full diff: https://github.com/llvm/llvm-project/pull/90717.diff

2 Files Affected:

• (modified) clang/lib/CodeGen/CodeGenFunction.h (+3-1)
• (added) clang/test/Profile/debug-info-instr_profile_switch.cpp (+40)

diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index ff1873325d409f..8a1c2ea0326c35 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1587,8 +1587,10 @@ class CodeGenFunction : public CodeGenTypeCache {
   void incrementProfileCounter(const Stmt *S, llvm::Value *StepV = nullptr) {
     if (CGM.getCodeGenOpts().hasProfileClangInstr() &&
         !CurFn->hasFnAttribute(llvm::Attribute::NoProfile) &&
-        !CurFn->hasFnAttribute(llvm::Attribute::SkipProfile))
+        !CurFn->hasFnAttribute(llvm::Attribute::SkipProfile)) {
+      auto AL = ApplyDebugLocation::CreateArtificial(*this);
       PGO.emitCounterSetOrIncrement(Builder, S, StepV);
+    }
     PGO.setCurrentStmt(S);
   }
 
diff --git a/clang/test/Profile/debug-info-instr_profile_switch.cpp b/clang/test/Profile/debug-info-instr_profile_switch.cpp
new file mode 100644
index 00000000000000..09edf9a96b0167
--- /dev/null
+++ b/clang/test/Profile/debug-info-instr_profile_switch.cpp
@@ -0,0 +1,40 @@
+// Tests that we don't attach misleading debug locations to llvm.instrprof.increment
+// counters.
+
+// RUN: %clang_cc1 -x c++ %s -debug-info-kind=standalone -triple %itanium_abi_triple -main-file-name debug-info-instr_profile_switch.cpp -std=c++11 -o - -emit-llvm -fprofile-instrument=clang | FileCheck %s
+
+int main(int argc, const char *argv[]) {
+  switch(argc) {
+    case 0:
+      return 0;
+    case 1:
+      return 1;
+  }
+}
+
+// CHECK: define noundef i32 @main(i32 noundef %argc, ptr noundef %argv) #0 !dbg ![[MAIN_SCOPE:[0-9]+]]
+
+// CHECK:        switch i32 {{.*}}, label {{.*}} [
+// CHECK-NEXT:     i32 0, label %[[CASE1_LBL:[a-z0-9.]+]]
+// CHECK-NEXT:     i32 1, label %[[CASE2_LBL:[a-z0-9.]+]]
+// CHECK-NEXT:   ], !dbg ![[SWITCH_LOC:[0-9]+]]
+
+// CHECK:       [[CASE1_LBL]]:
+// CHECK-NEXT:     %{{.*}} = load i64, ptr getelementptr inbounds ({{.*}}, ptr @__profc_main, {{.*}}), align {{.*}}, !dbg ![[CTR_LOC:[0-9]+]]
+// CHECK-NEXT:     %{{.*}} = add {{.*}}, !dbg ![[CTR_LOC]]
+// CHECK-NEXT:     store i64 {{.*}}, ptr getelementptr inbounds ({{.*}}, ptr @__profc_main, {{.*}}), align {{.*}}, !dbg ![[CTR_LOC]]
+// CHECK-NEXT:     store i32 0, {{.*}} !dbg ![[CASE1_LOC:[0-9]+]]
+// CHECK-NEXT:     br label {{.*}}, !dbg ![[CASE1_LOC]]
+
+// CHECK:       [[CASE2_LBL]]:
+// CHECK-NEXT:     %{{.*}} = load i64, ptr getelementptr inbounds ({{.*}}, ptr @__profc_main, {{.*}}), align {{.*}}, !dbg ![[CTR_LOC]]
+// CHECK-NEXT:     %{{.*}} = add {{.*}}, !dbg ![[CTR_LOC]]
+// CHECK-NEXT:     store i64 {{.*}}, ptr getelementptr inbounds ({{.*}}, ptr @__profc_main, {{.*}}), align {{.*}}, !dbg ![[CTR_LOC]]
+// CHECK-NEXT:     store i32 1, {{.*}} !dbg ![[CASE2_LOC:[0-9]+]]
+// CHECK-NEXT:     br label {{.*}}, !dbg ![[CASE2_LOC]]
+
+// CHECK: ![[SWITCH_LOC]] = !DILocation({{.*}}, scope: ![[MAIN_SCOPE]])
+// CHECK: ![[CTR_LOC]] = !DILocation(line: 0, scope: ![[BLOCK_SCOPE:[0-9]+]])
+// CHECK: ![[BLOCK_SCOPE]] = distinct !DILexicalBlock(scope: ![[MAIN_SCOPE]]
+// CHECK: ![[CASE1_LOC]] = !DILocation(line: {{.*}}, column: {{.*}}, scope: ![[BLOCK_SCOPE]])
+// CHECK: ![[CASE2_LOC]] = !DILocation(line: {{.*}}, column: {{.*}}, scope: ![[BLOCK_SCOPE]])

[Michael137]

Broke some build bots. Fixing...

[Michael137]

18707f53d6d2665634373847a0e9bdcbcac88c57