From cdf3eef4a2c3d46687b6b2e49d02b7e7152f2d5e Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams 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 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 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 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 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 };