[llvm-branch-commits] [clang] [KeyInstr][Clang] Assignment atom group (PR #134637)

Orlando Cazalet-Hyams via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Apr 7 06:47:50 PDT 2025


https://github.com/OCHyams created https://github.com/llvm/llvm-project/pull/134637

[KeyInstr][Clang] Assignment atom group

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

The Key Instructions project is introduced, including a "quick summary" section
at the top which adds context for this PR, here:
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.

The Clang-side work is demoed here:
https://github.com/llvm/llvm-project/pull/130943

[KeyInstr][Clang] Multiple assignment  (x = y = z)

Compound assign

Pre/Post Inc/Dec

Rename test & run C & C++

>From cdf3eef4a2c3d46687b6b2e49d02b7e7152f2d5e Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Wed, 2 Apr 2025 18:01:48 +0100
Subject: [PATCH 1/5] [KeyInstr][Clang] Assignment atom group

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

The Key Instructions project is introduced, including a "quick summary" section
at the top which adds context for this PR, here:
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.

The Clang-side work is demoed here:
https://github.com/llvm/llvm-project/pull/130943
---
 clang/lib/CodeGen/CGExpr.cpp          | 9 +++++++++
 clang/test/KeyInstructions/assign.cpp | 9 +++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 clang/test/KeyInstructions/assign.cpp

diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index a00a8ba2cd5cb..7408c498b3a1a 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5814,6 +5814,15 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const BinaryOperator *E) {
 
   assert(E->getOpcode() == BO_Assign && "unexpected binary l-value");
 
+  // This covers both LHS and RHS expressions, though nested RHS
+  // expressions may get subsequently separately grouped.
+  // FIXME(OCH): Not clear yet if we've got fine enough control
+  // to pick and choose when we need to. Currently looks ok:
+  // a = b = c  -> Two atoms.
+  // x = new(1) -> One atom (for both addr store and value store).
+  // 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/test/KeyInstructions/assign.cpp b/clang/test/KeyInstructions/assign.cpp
new file mode 100644
index 0000000000000..b09137430156f
--- /dev/null
+++ b/clang/test/KeyInstructions/assign.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang -gkey-instructions %s -gmlt -gcolumn-info -S -emit-llvm -o - -Wno-unused-variable \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+
+unsigned long long g;
+void fun() { g = 0; }
+
+// CHECK: store i64 0, ptr @g{{.*}}, !dbg [[G1R1:!.*]]
+
+// CHECK: [[G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)

>From b6f42ab8be5aee4c27e5f540b203e40eb7decc94 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Wed, 2 Apr 2025 18:27:59 +0100
Subject: [PATCH 2/5] [KeyInstr][Clang] Multiple assignment  (x = y = z)

---
 clang/lib/CodeGen/CGExprScalar.cpp    |  1 +
 clang/test/KeyInstructions/assign.cpp | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 140a12d384502..0019edf3e7552 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -4973,6 +4973,7 @@ llvm::Value *CodeGenFunction::EmitWithOriginalRHSBitfieldAssignment(
 }
 
 Value *ScalarExprEmitter::VisitBinAssign(const BinaryOperator *E) {
+  ApplyAtomGroup Grp(CGF.getDebugInfo());
   bool Ignore = TestAndClearIgnoreResultAssign();
 
   Value *RHS;
diff --git a/clang/test/KeyInstructions/assign.cpp b/clang/test/KeyInstructions/assign.cpp
index b09137430156f..d50062d21935b 100644
--- a/clang/test/KeyInstructions/assign.cpp
+++ b/clang/test/KeyInstructions/assign.cpp
@@ -2,8 +2,22 @@
 // RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
 
 unsigned long long g;
-void fun() { g = 0; }
-
+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;
+}
 
 // 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)

>From e92a5d1b0fe40a7961d8e8b822c4201be5bc6d8c Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Thu, 3 Apr 2025 11:05:29 +0100
Subject: [PATCH 3/5] Compound assign

---
 clang/lib/CodeGen/CGExprScalar.cpp                  |  2 ++
 .../KeyInstructions/{assign.cpp => assign-scalar.c} | 13 ++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)
 rename clang/test/KeyInstructions/{assign.cpp => assign-scalar.c} (58%)

diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 0019edf3e7552..929fa0471a233 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -897,6 +897,7 @@ class ScalarExprEmitter
     return result;                                                             \
   }                                                                            \
   Value *VisitBin##OP##Assign(const CompoundAssignOperator *E) {               \
+    ApplyAtomGroup Grp(CGF.getDebugInfo());                                    \
     return EmitCompoundAssign(E, &ScalarExprEmitter::Emit##OP);                \
   }
   HANDLEBINOP(Mul)
@@ -5741,6 +5742,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/KeyInstructions/assign.cpp b/clang/test/KeyInstructions/assign-scalar.c
similarity index 58%
rename from clang/test/KeyInstructions/assign.cpp
rename to clang/test/KeyInstructions/assign-scalar.c
index d50062d21935b..1e08195b06eb5 100644
--- a/clang/test/KeyInstructions/assign.cpp
+++ b/clang/test/KeyInstructions/assign-scalar.c
@@ -1,4 +1,7 @@
-// RUN: %clang -gkey-instructions %s -gmlt -gcolumn-info -S -emit-llvm -o - -Wno-unused-variable \
+// RUN: %clang -gkey-instructions -x c++ %s -gmlt -gcolumn-info -S -emit-llvm -o - -Wno-unused-variable \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+
+// RUN: %clang -gkey-instructions -x c %s -gmlt -gcolumn-info -S -emit-llvm -o - -Wno-unused-variable \
 // RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
 
 unsigned long long g;
@@ -15,9 +18,17 @@ void fun() {
 // 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;
 }
 
 // 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)

>From 800fde97f22093b9e0316ed4fe69662cfebd986a Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Thu, 3 Apr 2025 11:12:26 +0100
Subject: [PATCH 4/5] Pre/Post Inc/Dec

---
 clang/lib/CodeGen/CGExprScalar.cpp         |  1 +
 clang/test/KeyInstructions/assign-scalar.c | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 929fa0471a233..a4afafd3f536b 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2941,6 +2941,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;
diff --git a/clang/test/KeyInstructions/assign-scalar.c b/clang/test/KeyInstructions/assign-scalar.c
index 1e08195b06eb5..1f1fe8fda39e6 100644
--- a/clang/test/KeyInstructions/assign-scalar.c
+++ b/clang/test/KeyInstructions/assign-scalar.c
@@ -24,6 +24,16 @@ void fun() {
 // 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--;
 }
 
 // CHECK: [[G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
@@ -32,3 +42,7 @@ void fun() {
 // 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)

>From 678876993da1c71a12a63b8f48f04c0aa3e3a7e1 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Thu, 3 Apr 2025 11:27:18 +0100
Subject: [PATCH 5/5] Rename test & run C & C++

---
 clang/test/KeyInstructions/{init-agg.cpp => init-agg.c} | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
 rename clang/test/KeyInstructions/{init-agg.cpp => init-agg.c} (83%)

diff --git a/clang/test/KeyInstructions/init-agg.cpp b/clang/test/KeyInstructions/init-agg.c
similarity index 83%
rename from clang/test/KeyInstructions/init-agg.cpp
rename to clang/test/KeyInstructions/init-agg.c
index f0cc67e659d95..ad298715286cd 100644
--- a/clang/test/KeyInstructions/init-agg.cpp
+++ b/clang/test/KeyInstructions/init-agg.c
@@ -1,5 +1,8 @@
 
-// RUN: %clang -gkey-instructions %s -gmlt -gno-column-info -S -emit-llvm -o - -ftrivial-auto-var-init=pattern \
+// 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
@@ -7,7 +10,6 @@
 
 int g;
 void a() {
-// CHECK: _Z1av()
 // CHECK: call void @llvm.memcpy{{.*}}, !dbg [[G1R1:!.*]]
     int A[] = { 1, 2, 3 };
 



More information about the llvm-branch-commits mailing list