[clang] 8f1d1dd - [KeyInstr][Clang] Assignment atom group (#134637)

via cfe-commits cfe-commits at lists.llvm.org
Fri May 23 01:40:12 PDT 2025


Author: Orlando Cazalet-Hyams
Date: 2025-05-23T09:40:09+01:00
New Revision: 8f1d1dde8a9b4eb317bd515fcb4838f97adbf5f2

URL: https://github.com/llvm/llvm-project/commit/8f1d1dde8a9b4eb317bd515fcb4838f97adbf5f2
DIFF: https://github.com/llvm/llvm-project/commit/8f1d1dde8a9b4eb317bd515fcb4838f97adbf5f2.diff

LOG: [KeyInstr][Clang] Assignment atom group (#134637)

This patch is part of a stack that teaches Clang to generate Key Instructions
metadata for C and C++.

RFC:
https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668

The feature is only functional in LLVM if LLVM is built with CMake flag
LLVM_EXPERIMENTAL_KEY_INSTRUCTIONs. Eventually that flag will be removed.

Added: 
    clang/test/DebugInfo/KeyInstructions/assign-scalar.c
    clang/test/DebugInfo/KeyInstructions/init-agg.c

Modified: 
    clang/lib/CodeGen/CGExpr.cpp
    clang/lib/CodeGen/CGExprScalar.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 7cb7ee20fcf6a..2c0edd69d96e9 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5981,6 +5981,15 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const BinaryOperator *E) {
 
   assert(E->getOpcode() == BO_Assign && "unexpected binary l-value");
 
+  // Create a Key Instructions source location atom group that covers both
+  // LHS and RHS expressions. Nested RHS expressions may get subsequently
+  // separately grouped (1 below):
+  //
+  //   1. `a = b = c`  -> Two atoms.
+  //   2. `x = new(1)` -> One atom (for both addr store and value store).
+  //   3. Complex and agg assignment -> One atom.
+  ApplyAtomGroup Grp(getDebugInfo());
+
   // Note that in all of these cases, __block variables need the RHS
   // evaluated first just in case the variable gets moved by the RHS.
 

diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 5d618658bc615..b9eb12efd6670 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -898,6 +898,7 @@ class ScalarExprEmitter
     return result;                                                             \
   }                                                                            \
   Value *VisitBin##OP##Assign(const CompoundAssignOperator *E) {               \
+    ApplyAtomGroup Grp(CGF.getDebugInfo());                                    \
     return EmitCompoundAssign(E, &ScalarExprEmitter::Emit##OP);                \
   }
   HANDLEBINOP(Mul)
@@ -3014,6 +3015,7 @@ class OMPLastprivateConditionalUpdateRAII {
 llvm::Value *
 ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
                                            bool isInc, bool isPre) {
+  ApplyAtomGroup Grp(CGF.getDebugInfo());
   OMPLastprivateConditionalUpdateRAII OMPRegion(CGF, E);
   QualType type = E->getSubExpr()->getType();
   llvm::PHINode *atomicPHI = nullptr;
@@ -5067,6 +5069,7 @@ llvm::Value *CodeGenFunction::EmitWithOriginalRHSBitfieldAssignment(
 }
 
 Value *ScalarExprEmitter::VisitBinAssign(const BinaryOperator *E) {
+  ApplyAtomGroup Grp(CGF.getDebugInfo());
   bool Ignore = TestAndClearIgnoreResultAssign();
 
   Value *RHS;
@@ -5849,6 +5852,7 @@ LValue CodeGenFunction::EmitObjCIsaExpr(const ObjCIsaExpr *E) {
 
 LValue CodeGenFunction::EmitCompoundAssignmentLValue(
                                             const CompoundAssignOperator *E) {
+  ApplyAtomGroup Grp(getDebugInfo());
   ScalarExprEmitter Scalar(*this);
   Value *Result = nullptr;
   switch (E->getOpcode()) {

diff  --git a/clang/test/DebugInfo/KeyInstructions/assign-scalar.c b/clang/test/DebugInfo/KeyInstructions/assign-scalar.c
new file mode 100644
index 0000000000000..98d005b2e925c
--- /dev/null
+++ b/clang/test/DebugInfo/KeyInstructions/assign-scalar.c
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1 -gkey-instructions -x c++ %s -debug-info-kind=line-tables-only -emit-llvm -o - \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+
+// RUN: %clang_cc1 -gkey-instructions -x c %s -debug-info-kind=line-tables-only -emit-llvm -o - \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+
+unsigned long long g, h, i;
+void fun() {
+// CHECK: store i64 0, ptr @g{{.*}}, !dbg [[G1R1:!.*]]
+    g = 0;
+
+// Treat the two assignments as two atoms.
+//
+// FIXME: Because of the atomGroup implementation the load can only be
+// associated with one of the two stores, despite being a good backup
+// loction for both.
+// CHECK-NEXT: %0 = load i64, ptr @g{{.*}}, !dbg [[G2R2:!.*]]
+// CHECK-NEXT: store i64 %0, ptr @g{{.*}}, !dbg [[G3R1:!.*]]
+// CHECK-NEXT: store i64 %0, ptr @g{{.*}}, !dbg [[G2R1:!.*]]
+    g = g = g;
+
+// Compound assignment.
+// CHECK: %1 = load i64, ptr @g
+// CHECK: %add = add i64 %1, 50, !dbg [[G4R2:!.*]]
+// CHECK: store i64 %add, ptr @g{{.*}}, !dbg [[G4R1:!.*]]
+    g += 50;
+
+// Pre/Post Inc/Dec.
+// CHECK: %2 = load i64, ptr @g
+// CHECK: %inc = add i64 %2, 1, !dbg [[G5R2:!.*]]
+// CHECK: store i64 %inc, ptr @g{{.*}}, !dbg [[G5R1:!.*]]
+    ++g;
+// CHECK: %3 = load i64, ptr @g
+// CHECK: %dec = add i64 %3, -1, !dbg [[G6R2:!.*]]
+// CHECK: store i64 %dec, ptr @g{{.*}}, !dbg [[G6R1:!.*]]
+    g--;
+
+// Compound assignment with assignment on RHS, the assignments should have
+// their own separate atom groups.
+// CHECK-NEXT: %4 = load i64, ptr @h{{.*}}, !dbg [[load_h_loc:!.*]]
+// CHECK-NEXT: %inc1 = add i64 %4, 1, !dbg [[G8R2:!.*]]
+// CHECK-NEXT: store i64 %inc1, ptr @h{{.*}}, !dbg [[G8R1:!.*]]
+// CHECK-NEXT: %5 = load i64, ptr @g{{.*}}, !dbg [[load_g_loc:!.*]]
+// CHECK-NEXT: %add2 = add i64 %5, %inc1, !dbg [[G7R2:!.*]]
+// CHECK-NEXT: store i64 %add2, ptr @g{{.*}}, !dbg [[G7R1:!.*]]
+    g += ++h;
+
+// Double check the comma operator doesn't disturb atom groupings. There
+// are three assignments, so we should get three groups.
+// FIXME: Same situation as earlier in the test - because of the atomGroup
+// implementation the load (from h) can only be associated with one of the two
+// stores (to h and g) despite being a good backup location for both.
+// CHECK-NEXT: %6 = load i64, ptr @h{{.*}}, !dbg [[load_h_loc2:!.*]]
+// CHECK-NEXT: %inc3 = add i64 %6, 1, !dbg [[G9R2:!.*]]
+// CHECK-NEXT: store i64 %inc3, ptr @h{{.*}}, !dbg [[G10R1:!.*]]
+// CHECK-NEXT: store i64 %inc3, ptr @g{{.*}}, !dbg [[G9R1:!.*]]
+// CHECK-NEXT: %7 = load i64, ptr @i{{.*}}, !dbg [[load_i_loc:!.*]]
+// CHECK-NEXT: %inc4 = add i64 %7, 1, !dbg [[G11R2:!.*]]
+// CHECK-NEXT: store i64 %inc4, ptr @i{{.*}}, !dbg [[G11R1:!.*]]
+    g = ++h, ++i;
+}
+
+// CHECK: [[G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
+// CHECK: [[G2R2]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 2)
+// CHECK: [[G3R1]] = !DILocation({{.*}}, atomGroup: 3, atomRank: 1)
+// CHECK: [[G2R1]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 1)
+// CHECK: [[G4R2]] = !DILocation({{.*}}, atomGroup: 4, atomRank: 2)
+// CHECK: [[G4R1]] = !DILocation({{.*}}, atomGroup: 4, atomRank: 1)
+// CHECK: [[G5R2]] = !DILocation({{.*}}, atomGroup: 5, atomRank: 2)
+// CHECK: [[G5R1]] = !DILocation({{.*}}, atomGroup: 5, atomRank: 1)
+// CHECK: [[G6R2]] = !DILocation({{.*}}, atomGroup: 6, atomRank: 2)
+// CHECK: [[G6R1]] = !DILocation({{.*}}, atomGroup: 6, atomRank: 1)
+// CHECK: [[load_h_loc]] = !DILocation(line: [[#]], column: [[#]], scope: ![[#]])
+// CHECK: [[G8R2]] = !DILocation({{.*}}, atomGroup: 8, atomRank: 2)
+// CHECK: [[G8R1]] = !DILocation({{.*}}, atomGroup: 8, atomRank: 1)
+// CHECK: [[load_g_loc]] = !DILocation(line: [[#]], column: [[#]], scope: ![[#]])
+// CHECK: [[G7R2]] = !DILocation({{.*}}, atomGroup: 7, atomRank: 2)
+// CHECK: [[G7R1]] = !DILocation({{.*}}, atomGroup: 7, atomRank: 1)
+// CHECK: [[load_h_loc2]] = !DILocation(line: [[#]], column: [[#]], scope: ![[#]])
+// CHECK: [[G9R2]] = !DILocation({{.*}}, atomGroup: 9, atomRank: 2)
+// CHECK: [[G10R1]] = !DILocation({{.*}}, atomGroup: 10, atomRank: 1)
+// CHECK: [[G9R1]] = !DILocation({{.*}}, atomGroup: 9, atomRank: 1)
+// CHECK: [[load_i_loc]] = !DILocation(line: [[#]], column: [[#]], scope: ![[#]])
+// CHECK: [[G11R2]] = !DILocation({{.*}}, atomGroup: 11, atomRank: 2)
+// CHECK: [[G11R1]] = !DILocation({{.*}}, atomGroup: 11, atomRank: 1)

diff  --git a/clang/test/DebugInfo/KeyInstructions/init-agg.c b/clang/test/DebugInfo/KeyInstructions/init-agg.c
new file mode 100644
index 0000000000000..21b26092e5acf
--- /dev/null
+++ b/clang/test/DebugInfo/KeyInstructions/init-agg.c
@@ -0,0 +1,41 @@
+
+// RUN: %clang -gkey-instructions -x c++ %s -gmlt -gno-column-info -S -emit-llvm -o - -ftrivial-auto-var-init=pattern \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+
+// RUN: %clang -gkey-instructions -x c %s -gmlt -gno-column-info -S -emit-llvm -o - -ftrivial-auto-var-init=pattern \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+
+// The implicit-check-not is important; we don't want the GEPs created for the
+// store locations to be included in the atom group.
+
+int g;
+void a() {
+// CHECK: call void @llvm.memcpy{{.*}}%A{{.*}}, !dbg [[G1R1:!.*]]
+    int A[] = { 1, 2, 3 };
+
+// CHECK: store i32 1, ptr %{{.*}}, !dbg [[G2R1:!.*]]
+// CHECK: store i32 2, ptr %{{.*}}, !dbg [[G2R1]]
+// CHECK: %0 = load i32, ptr @g{{.*}}, !dbg [[G2R2:!.*]]
+// CHECK: store i32 %0, ptr %{{.*}}, !dbg [[G2R1]]
+    int B[] = { 1, 2, g };
+
+// CHECK: call void @llvm.memset{{.*}}%big{{.*}}, !dbg [[G3R1:!.*]]
+// CHECK: store i8 97{{.*}}, !dbg [[G3R1]]
+// CHECK: store i8 98{{.*}}, !dbg [[G3R1]]
+// CHECK: store i8 99{{.*}}, !dbg [[G3R1]]
+// CHECK: store i8 100{{.*}}, !dbg [[G3R1]]
+    char big[65536] = { 'a', 'b', 'c', 'd' };
+
+// CHECK: call void @llvm.memset{{.*}}%arr{{.*}}, !dbg [[G4R1:!.*]]
+    char arr[] = { 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, };
+
+// CHECK: store i8 -86, ptr %uninit{{.*}}, !dbg [[G5R1:!.*]], !annotation
+    char uninit; // -ftrivial-auto-var-init=pattern
+}
+
+// CHECK: [[G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
+// CHECK: [[G2R1]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 1)
+// CHECK: [[G2R2]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 2)
+// CHECK: [[G3R1]] = !DILocation({{.*}}, atomGroup: 3, atomRank: 1)
+// CHECK: [[G4R1]] = !DILocation({{.*}}, atomGroup: 4, atomRank: 1)
+// CHECK: [[G5R1]] = !DILocation({{.*}}, atomGroup: 5, atomRank: 1)


        


More information about the cfe-commits mailing list