Skip to content

[KeyInstr][Clang] Atomic ops atoms #141624

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 24, 2025
Merged

[KeyInstr][Clang] Atomic ops atoms #141624

merged 1 commit into from
Jun 24, 2025

Conversation

OCHyams
Copy link
Contributor

@OCHyams OCHyams commented May 27, 2025

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

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

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

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

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

RFC:
https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668
@OCHyams OCHyams requested review from jmorse and SLTozer May 27, 2025 15:51
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels May 27, 2025
@llvmbot
Copy link
Member

llvmbot commented May 27, 2025

@llvm/pr-subscribers-clang

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

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

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

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


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGAtomic.cpp (+13-4)
  • (added) clang/test/DebugInfo/KeyInstructions/atomic.c (+162)
diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index 51f0799a792fd..9106c4cd8e139 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -411,7 +411,9 @@ static void emitAtomicCmpXchg(CodeGenFunction &CGF, AtomicExpr *E, bool IsWeak,
 
   CGF.Builder.SetInsertPoint(StoreExpectedBB);
   // Update the memory at Expected with Old's value.
-  CGF.Builder.CreateStore(Old, Val1);
+  auto *I = CGF.Builder.CreateStore(Old, Val1);
+  CGF.addInstToCurrentSourceAtom(I, Old);
+
   // Finally, branch to the exit point.
   CGF.Builder.CreateBr(ContinueBB);
 
@@ -591,7 +593,8 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
     Load->setAtomic(Order, Scope);
     Load->setVolatile(E->isVolatile());
     CGF.maybeAttachRangeForLoad(Load, E->getValueType(), E->getExprLoc());
-    CGF.Builder.CreateStore(Load, Dest);
+    auto *I = CGF.Builder.CreateStore(Load, Dest);
+    CGF.addInstToCurrentSourceAtom(I, Load);
     return;
   }
 
@@ -606,6 +609,7 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
     llvm::StoreInst *Store = CGF.Builder.CreateStore(LoadVal1, Ptr);
     Store->setAtomic(Order, Scope);
     Store->setVolatile(E->isVolatile());
+    CGF.addInstToCurrentSourceAtom(Store, LoadVal1);
     return;
   }
 
@@ -731,7 +735,8 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
                               CGF.Builder.getInt8(1), Order, Scope, E);
     RMWI->setVolatile(E->isVolatile());
     llvm::Value *Result = CGF.Builder.CreateIsNotNull(RMWI, "tobool");
-    CGF.Builder.CreateStore(Result, Dest);
+    auto *I = CGF.Builder.CreateStore(Result, Dest);
+    CGF.addInstToCurrentSourceAtom(I, Result);
     return;
   }
 
@@ -740,6 +745,7 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
         CGF.Builder.CreateStore(CGF.Builder.getInt8(0), Ptr);
     Store->setAtomic(Order, Scope);
     Store->setVolatile(E->isVolatile());
+    CGF.addInstToCurrentSourceAtom(Store, nullptr);
     return;
   }
   }
@@ -762,7 +768,8 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
   if (E->getOp() == AtomicExpr::AO__atomic_nand_fetch ||
       E->getOp() == AtomicExpr::AO__scoped_atomic_nand_fetch)
     Result = CGF.Builder.CreateNot(Result);
-  CGF.Builder.CreateStore(Result, Dest);
+  auto *I = CGF.Builder.CreateStore(Result, Dest);
+  CGF.addInstToCurrentSourceAtom(I, Result);
 }
 
 // This function emits any expression (scalar, complex, or aggregate)
@@ -845,6 +852,8 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *Expr, Address Dest,
 }
 
 RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
+  ApplyAtomGroup Grp(getDebugInfo());
+
   QualType AtomicTy = E->getPtr()->getType()->getPointeeType();
   QualType MemTy = AtomicTy;
   if (const AtomicType *AT = AtomicTy->getAs<AtomicType>())
diff --git a/clang/test/DebugInfo/KeyInstructions/atomic.c b/clang/test/DebugInfo/KeyInstructions/atomic.c
new file mode 100644
index 0000000000000..da54e37714c3a
--- /dev/null
+++ b/clang/test/DebugInfo/KeyInstructions/atomic.c
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -gkey-instructions -x c++ -std=c++17 %s -debug-info-kind=line-tables-only -emit-llvm -o - -gno-column-info \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -gkey-instructions -x c %s -debug-info-kind=line-tables-only -emit-llvm -o - -gno-column-info \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+
+// Check that atomic handling code gets Key Instruction annotations.
+
+_Atomic(unsigned int) x;
+unsigned int y;
+void fun() {
+  unsigned int r1 = __c11_atomic_fetch_add(&x,- 1, __ATOMIC_RELAXED);
+// CHECK:      store i32 -1, ptr %.atomictmp, align 4,                !dbg [[LINE11_G2R1:!.*]]
+// CHECK-NEXT: %0 = load i32, ptr %.atomictmp, align 4,               !dbg [[LINE11:!.*]]
+// CHECK-NEXT: %1 = atomicrmw add ptr @x, i32 %0 monotonic, align 4,  !dbg [[LINE11_G2R2:!.*]]
+// CHECK-NEXT: store i32 %1, ptr %atomic-temp, align 4,               !dbg [[LINE11_G2R1]]
+// CHECK-NEXT: %2 = load i32, ptr %atomic-temp, align 4,              !dbg [[LINE11_G1R2:!.*]]
+// CHECK-NEXT: store i32 %2, ptr %r1, align 4,                        !dbg [[LINE11_G1R1:!.*]]
+
+  unsigned int r2 = __c11_atomic_load(&x, __ATOMIC_RELAXED);
+// CHECK-NEXT: %3 = load atomic i32, ptr @x monotonic, align 4,       !dbg [[LINE19_G4R2:!.*]]
+// CHECK-NEXT: store i32 %3, ptr %atomic-temp1, align 4,              !dbg [[LINE19_G4R1:!.*]]
+// CHECK-NEXT: %4 = load i32, ptr %atomic-temp1, align 4,             !dbg [[LINE19_G3R2:!.*]]
+// CHECK-NEXT: store i32 %4, ptr %r2, align 4,                        !dbg [[LINE19_G3R1:!.*]]
+
+  __c11_atomic_store(&x, 2, __ATOMIC_RELAXED);
+// CHECK-NEXT: store i32 2, ptr %.atomictmp2, align 4,                !dbg [[LINE25_G5R1:!.*]]
+// CHECK-NEXT: %5 = load i32, ptr %.atomictmp2, align 4,              !dbg [[LINE25_G5R2:!.*]]
+// CHECK-NEXT: store atomic i32 %5, ptr @x monotonic, align 4,        !dbg [[LINE25_G5R1:!.*]]
+
+  int r3 = __atomic_test_and_set(&x, __ATOMIC_RELAXED);
+// CHECK-NEXT: %6 = atomicrmw xchg ptr @x, i8 1 monotonic, align 4,   !dbg [[LINE30:!.*]]
+// CHECK-NEXT: %tobool = icmp ne i8 %6, 0,                            !dbg [[LINE30_G7R2:!.*]]
+// CHECK-NEXT: store i1 %tobool, ptr %atomic-temp3, align 1,          !dbg [[LINE30_G7R1:!.*]]
+// CHECK-NEXT: %7 = load i8, ptr %atomic-temp3, align 1,              !dbg [[LINE30_G6R4:!.*]]
+// CHECK-NEXT: %loadedv = trunc i8 %7 to i1,                          !dbg [[LINE30_G6R3:!.*]]
+// CHECK-NEXT: %conv = zext i1 %loadedv to i32,                       !dbg [[LINE30_G6R2:!.*]]
+// CHECK-NEXT: store i32 %conv, ptr %r3, align 4,                     !dbg [[LINE30_G6R1:!.*]]
+
+  __atomic_clear(&x, __ATOMIC_RELAXED);
+// CHECK-NEXT: store atomic i8 0, ptr @x monotonic, align 4,          !dbg [[LINE39_G8R1:!.*]]
+
+  int r4 = __c11_atomic_exchange(&x, 2,__ATOMIC_RELAXED);
+// CHECK-NEXT: store i32 2, ptr %.atomictmp4, align 4,                !dbg [[LINE42_G10R1:!.*]]
+// CHECK-NEXT: %8 = load i32, ptr %.atomictmp4, align 4,              !dbg [[LINE42:!.*]]
+// CHECK-NEXT: %9 = atomicrmw xchg ptr @x, i32 %8 monotonic, align 4, !dbg [[LINE42_G10R2:!.*]]
+// CHECK-NEXT: store i32 %9, ptr %atomic-temp5, align 4,              !dbg [[LINE42_G10R1:!.*]]
+// CHECK-NEXT: %10 = load i32, ptr %atomic-temp5, align 4,            !dbg [[LINE42_G9R2:!.*]]
+// CHECK-NEXT: store i32 %10, ptr %r4, align 4,                       !dbg [[LINE42_G9R1:!.*]]
+
+  int r5 = __atomic_compare_exchange(&y, &y, &y, 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
+// CHECK-NEXT: %11 = load i32, ptr @y, align 4,                       !dbg [[LINE50:!.*]]
+// CHECK-NEXT: %12 = load i32, ptr @y, align 4,                       !dbg [[LINE50]]
+// CHECK-NEXT: %13 = cmpxchg ptr @y, i32 %11, i32 %12 monotonic monotonic, align 4, !dbg [[LINE50]]
+// CHECK-NEXT: %14 = extractvalue { i32, i1 } %13, 0,                 !dbg [[LINE50_G12R2:!.*]]
+// CHECK-NEXT: %15 = extractvalue { i32, i1 } %13, 1,                 !dbg [[LINE50_G12R3:!.*]]
+// CHECK-NEXT: br i1 %15, label %cmpxchg.continue, label %cmpxchg.store_expected, !dbg [[LINE50]]
+// CHECK: cmpxchg.store_expected:
+// CHECK-NEXT: store i32 %14, ptr @y, align 4,                        !dbg [[LINE50_G12R1:!.*]]
+// CHECK-NEXT: br label %cmpxchg.continue,                            !dbg [[LINE50]]
+// CHECK: cmpxchg.continue:
+// CHECK-NEXT: %storedv = zext i1 %15 to i8,                          !dbg [[LINE50_G12R2]]
+// CHECK-NEXT: store i8 %storedv, ptr %cmpxchg.bool, align 1,         !dbg [[LINE50_G12R1]]
+// CHECK-NEXT: %16 = load i8, ptr %cmpxchg.bool, align 1,             !dbg [[LINE50_G11R4:!.*]]
+// CHECK-NEXT: %loadedv6 = trunc i8 %16 to i1,                        !dbg [[LINE50_G11R3:!.*]]
+// CHECK-NEXT: %conv7 = zext i1 %loadedv6 to i32,                     !dbg [[LINE50_G11R2:!.*]]
+// CHECK-NEXT: store i32 %conv7, ptr %r5, align 4,                    !dbg [[LINE50_G11R1:!.*]]
+
+  int r6 = __c11_atomic_compare_exchange_strong(&x, &y, 42, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
+// CHECK-NEXT: store i32 42, ptr %.atomictmp8, align 4,               !dbg [[LINE68_G14R1:!.*]]
+// CHECK-NEXT: %17 = load i32, ptr @y, align 4,                       !dbg [[LINE68:!.*]]
+// CHECK-NEXT: %18 = load i32, ptr %.atomictmp8, align 4,             !dbg [[LINE68]]
+// CHECK-NEXT: %19 = cmpxchg ptr @x, i32 %17, i32 %18 monotonic monotonic, align 4, !dbg [[LINE68]]
+// CHECK-NEXT: %20 = extractvalue { i32, i1 } %19, 0,                 !dbg [[LINE68_G14R2:!.*]]
+// CHECK-NEXT: %21 = extractvalue { i32, i1 } %19, 1,                 !dbg [[LINE68_G14R3:!.*]]
+// CHECK-NEXT: br i1 %21, label %cmpxchg.continue11, label %cmpxchg.store_expected10, !dbg [[LINE68]]
+// CHECK: cmpxchg.store_expected10:
+// CHECK-NEXT: store i32 %20, ptr @y, align 4,                        !dbg [[LINE68_G14R1:!.*]]
+// CHECK-NEXT: br label %cmpxchg.continue11,                          !dbg [[LINE68]]
+// CHECK: cmpxchg.continue11:
+// CHECK-NEXT: %storedv12 = zext i1 %21 to i8,                        !dbg [[LINE68_G14R2]]
+// CHECK-NEXT: store i8 %storedv12, ptr %cmpxchg.bool9, align 1,      !dbg [[LINE68_G14R1:!.*]]
+// CHECK-NEXT: %22 = load i8, ptr %cmpxchg.bool9, align 1,            !dbg [[LINE68_G13R4:!.*]]
+// CHECK-NEXT: %loadedv13 = trunc i8 %22 to i1,                       !dbg [[LINE68_G13R3:!.*]]
+// CHECK-NEXT: %conv14 = zext i1 %loadedv13 to i32,                   !dbg [[LINE68_G13R2:!.*]]
+// CHECK-NEXT: store i32 %conv14, ptr %r6, align 4,                   !dbg [[LINE68_G13R1:!.*]]
+
+  int r7 = __c11_atomic_compare_exchange_weak(&x, &y, 43, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
+// CHECK-NEXT: store i32 43, ptr %.atomictmp15, align 4,              !dbg [[LINE87_G16R1:!.*]]
+// CHECK-NEXT: %23 = load i32, ptr @y, align 4,                       !dbg [[LINE87:!.*]]
+// CHECK-NEXT: %24 = load i32, ptr %.atomictmp15, align 4,            !dbg [[LINE87]]
+// CHECK-NEXT: %25 = cmpxchg weak ptr @x, i32 %23, i32 %24 monotonic monotonic, align 4, !dbg [[LINE87]]
+// CHECK-NEXT: %26 = extractvalue { i32, i1 } %25, 0,                 !dbg [[LINE87_G16R2:!.*]]
+// CHECK-NEXT: %27 = extractvalue { i32, i1 } %25, 1,                 !dbg [[LINE87_G16R3:!.*]]
+// CHECK-NEXT: br i1 %27, label %cmpxchg.continue18, label %cmpxchg.store_expected17, !dbg [[LINE87]]
+// CHECK: cmpxchg.store_expected17:
+// CHECK-NEXT: store i32 %26, ptr @y, align 4,                        !dbg [[LINE87_G16R1]]
+// CHECK-NEXT: br label %cmpxchg.continue18,                          !dbg [[LINE87]]
+// CHECK: cmpxchg.continue18:
+// CHECK-NEXT: %storedv19 = zext i1 %27 to i8,                        !dbg [[LINE87_G16R2]]
+// CHECK-NEXT: store i8 %storedv19, ptr %cmpxchg.bool16, align 1,     !dbg [[LINE87_G16R1]]
+// CHECK-NEXT: %28 = load i8, ptr %cmpxchg.bool16, align 1,           !dbg [[LINE87_G15R4:!.*]]
+// CHECK-NEXT: %loadedv20 = trunc i8 %28 to i1,                       !dbg [[LINE87_G15R3:!.*]]
+// CHECK-NEXT: %conv21 = zext i1 %loadedv20 to i32,                   !dbg [[LINE87_G15R2:!.*]]
+// CHECK-NEXT: store i32 %conv21, ptr %r7, align 4,                   !dbg [[LINE87_G15R1:!.*]]
+}
+
+// CHECK: [[LINE11_G2R1]] = !DILocation(line: 11, scope: ![[#]], atomGroup: 2, atomRank: 1)
+// CHECK: [[LINE11]]      = !DILocation(line: 11, scope: ![[#]])
+// CHECK: [[LINE11_G2R2]] = !DILocation(line: 11, scope: ![[#]], atomGroup: 2, atomRank: 2)
+// CHECK: [[LINE11_G1R2]] = !DILocation(line: 11, scope: ![[#]], atomGroup: 1, atomRank: 2)
+// CHECK: [[LINE11_G1R1]] = !DILocation(line: 11, scope: ![[#]], atomGroup: 1, atomRank: 1)
+
+// CHECK: [[LINE19_G4R2]] = !DILocation(line: 19, scope: ![[#]], atomGroup: 4, atomRank: 2)
+// CHECK: [[LINE19_G4R1]] = !DILocation(line: 19, scope: ![[#]], atomGroup: 4, atomRank: 1)
+// CHECK: [[LINE19_G3R2]] = !DILocation(line: 19, scope: ![[#]], atomGroup: 3, atomRank: 2)
+// CHECK: [[LINE19_G3R1]] = !DILocation(line: 19, scope: ![[#]], atomGroup: 3, atomRank: 1)
+
+// CHECK: [[LINE25_G5R1]] = !DILocation(line: 25, scope: ![[#]], atomGroup: 5, atomRank: 1)
+// CHECK: [[LINE25_G5R2]] = !DILocation(line: 25, scope: ![[#]], atomGroup: 5, atomRank: 2)
+
+// CHECK: [[LINE30]]      = !DILocation(line: 30, scope: ![[#]])
+// CHECK: [[LINE30_G7R2]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 7, atomRank: 2)
+// CHECK: [[LINE30_G7R1]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 7, atomRank: 1)
+// CHECK: [[LINE30_G6R4]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 6, atomRank: 4)
+// CHECK: [[LINE30_G6R3]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 6, atomRank: 3)
+// CHECK: [[LINE30_G6R2]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 6, atomRank: 2)
+// CHECK: [[LINE30_G6R1]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 6, atomRank: 1)
+
+// CHECK: [[LINE39_G8R1]] = !DILocation(line: 39, scope: ![[#]], atomGroup: 8, atomRank: 1)
+
+// CHECK: [[LINE42_G10R1]] = !DILocation(line: 42, scope: ![[#]], atomGroup: 10, atomRank: 1)
+// CHECK: [[LINE42]]       = !DILocation(line: 42, scope: ![[#]])
+// CHECK: [[LINE42_G10R2]] = !DILocation(line: 42, scope: ![[#]], atomGroup: 10, atomRank: 2)
+// CHECK: [[LINE42_G9R2]]  = !DILocation(line: 42, scope: ![[#]], atomGroup: 9, atomRank: 2)
+// CHECK: [[LINE42_G9R1]]  = !DILocation(line: 42, scope: ![[#]], atomGroup: 9, atomRank: 1)
+
+// CHECK: [[LINE50]]       = !DILocation(line: 50, scope: ![[#]])
+// CHECK: [[LINE50_G12R2]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 12, atomRank: 2)
+// CHECK: [[LINE50_G12R3]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 12, atomRank: 3)
+// CHECK: [[LINE50_G12R1]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 12, atomRank: 1)
+// CHECK: [[LINE50_G11R4]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 11, atomRank: 4)
+// CHECK: [[LINE50_G11R3]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 11, atomRank: 3)
+// CHECK: [[LINE50_G11R2]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 11, atomRank: 2)
+// CHECK: [[LINE50_G11R1]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 11, atomRank: 1)
+
+// CHECK: [[LINE68_G14R1]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 14, atomRank: 1)
+// CHECK: [[LINE68]]       = !DILocation(line: 68, scope: ![[#]])
+// CHECK: [[LINE68_G14R2]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 14, atomRank: 2)
+// CHECK: [[LINE68_G14R3]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 14, atomRank: 3)
+// CHECK: [[LINE68_G13R4]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 13, atomRank: 4)
+// CHECK: [[LINE68_G13R3]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 13, atomRank: 3)
+// CHECK: [[LINE68_G13R2]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 13, atomRank: 2)
+// CHECK: [[LINE68_G13R1]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 13, atomRank: 1)
+
+// CHECK: [[LINE87_G16R1]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 16, atomRank: 1)
+// CHECK: [[LINE87]]       = !DILocation(line: 87, scope: ![[#]])
+// CHECK: [[LINE87_G16R2]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 16, atomRank: 2)
+// CHECK: [[LINE87_G16R3]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 16, atomRank: 3)
+// CHECK: [[LINE87_G15R4]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 15, atomRank: 4)
+// CHECK: [[LINE87_G15R3]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 15, atomRank: 3)
+// CHECK: [[LINE87_G15R2]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 15, atomRank: 2)
+// CHECK: [[LINE87_G15R1]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 15, atomRank: 1)

@llvmbot
Copy link
Member

llvmbot commented May 27, 2025

@llvm/pr-subscribers-clang-codegen

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

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

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

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


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGAtomic.cpp (+13-4)
  • (added) clang/test/DebugInfo/KeyInstructions/atomic.c (+162)
diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index 51f0799a792fd..9106c4cd8e139 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -411,7 +411,9 @@ static void emitAtomicCmpXchg(CodeGenFunction &CGF, AtomicExpr *E, bool IsWeak,
 
   CGF.Builder.SetInsertPoint(StoreExpectedBB);
   // Update the memory at Expected with Old's value.
-  CGF.Builder.CreateStore(Old, Val1);
+  auto *I = CGF.Builder.CreateStore(Old, Val1);
+  CGF.addInstToCurrentSourceAtom(I, Old);
+
   // Finally, branch to the exit point.
   CGF.Builder.CreateBr(ContinueBB);
 
@@ -591,7 +593,8 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
     Load->setAtomic(Order, Scope);
     Load->setVolatile(E->isVolatile());
     CGF.maybeAttachRangeForLoad(Load, E->getValueType(), E->getExprLoc());
-    CGF.Builder.CreateStore(Load, Dest);
+    auto *I = CGF.Builder.CreateStore(Load, Dest);
+    CGF.addInstToCurrentSourceAtom(I, Load);
     return;
   }
 
@@ -606,6 +609,7 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
     llvm::StoreInst *Store = CGF.Builder.CreateStore(LoadVal1, Ptr);
     Store->setAtomic(Order, Scope);
     Store->setVolatile(E->isVolatile());
+    CGF.addInstToCurrentSourceAtom(Store, LoadVal1);
     return;
   }
 
@@ -731,7 +735,8 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
                               CGF.Builder.getInt8(1), Order, Scope, E);
     RMWI->setVolatile(E->isVolatile());
     llvm::Value *Result = CGF.Builder.CreateIsNotNull(RMWI, "tobool");
-    CGF.Builder.CreateStore(Result, Dest);
+    auto *I = CGF.Builder.CreateStore(Result, Dest);
+    CGF.addInstToCurrentSourceAtom(I, Result);
     return;
   }
 
@@ -740,6 +745,7 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
         CGF.Builder.CreateStore(CGF.Builder.getInt8(0), Ptr);
     Store->setAtomic(Order, Scope);
     Store->setVolatile(E->isVolatile());
+    CGF.addInstToCurrentSourceAtom(Store, nullptr);
     return;
   }
   }
@@ -762,7 +768,8 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
   if (E->getOp() == AtomicExpr::AO__atomic_nand_fetch ||
       E->getOp() == AtomicExpr::AO__scoped_atomic_nand_fetch)
     Result = CGF.Builder.CreateNot(Result);
-  CGF.Builder.CreateStore(Result, Dest);
+  auto *I = CGF.Builder.CreateStore(Result, Dest);
+  CGF.addInstToCurrentSourceAtom(I, Result);
 }
 
 // This function emits any expression (scalar, complex, or aggregate)
@@ -845,6 +852,8 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *Expr, Address Dest,
 }
 
 RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
+  ApplyAtomGroup Grp(getDebugInfo());
+
   QualType AtomicTy = E->getPtr()->getType()->getPointeeType();
   QualType MemTy = AtomicTy;
   if (const AtomicType *AT = AtomicTy->getAs<AtomicType>())
diff --git a/clang/test/DebugInfo/KeyInstructions/atomic.c b/clang/test/DebugInfo/KeyInstructions/atomic.c
new file mode 100644
index 0000000000000..da54e37714c3a
--- /dev/null
+++ b/clang/test/DebugInfo/KeyInstructions/atomic.c
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -gkey-instructions -x c++ -std=c++17 %s -debug-info-kind=line-tables-only -emit-llvm -o - -gno-column-info \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -gkey-instructions -x c %s -debug-info-kind=line-tables-only -emit-llvm -o - -gno-column-info \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+
+// Check that atomic handling code gets Key Instruction annotations.
+
+_Atomic(unsigned int) x;
+unsigned int y;
+void fun() {
+  unsigned int r1 = __c11_atomic_fetch_add(&x,- 1, __ATOMIC_RELAXED);
+// CHECK:      store i32 -1, ptr %.atomictmp, align 4,                !dbg [[LINE11_G2R1:!.*]]
+// CHECK-NEXT: %0 = load i32, ptr %.atomictmp, align 4,               !dbg [[LINE11:!.*]]
+// CHECK-NEXT: %1 = atomicrmw add ptr @x, i32 %0 monotonic, align 4,  !dbg [[LINE11_G2R2:!.*]]
+// CHECK-NEXT: store i32 %1, ptr %atomic-temp, align 4,               !dbg [[LINE11_G2R1]]
+// CHECK-NEXT: %2 = load i32, ptr %atomic-temp, align 4,              !dbg [[LINE11_G1R2:!.*]]
+// CHECK-NEXT: store i32 %2, ptr %r1, align 4,                        !dbg [[LINE11_G1R1:!.*]]
+
+  unsigned int r2 = __c11_atomic_load(&x, __ATOMIC_RELAXED);
+// CHECK-NEXT: %3 = load atomic i32, ptr @x monotonic, align 4,       !dbg [[LINE19_G4R2:!.*]]
+// CHECK-NEXT: store i32 %3, ptr %atomic-temp1, align 4,              !dbg [[LINE19_G4R1:!.*]]
+// CHECK-NEXT: %4 = load i32, ptr %atomic-temp1, align 4,             !dbg [[LINE19_G3R2:!.*]]
+// CHECK-NEXT: store i32 %4, ptr %r2, align 4,                        !dbg [[LINE19_G3R1:!.*]]
+
+  __c11_atomic_store(&x, 2, __ATOMIC_RELAXED);
+// CHECK-NEXT: store i32 2, ptr %.atomictmp2, align 4,                !dbg [[LINE25_G5R1:!.*]]
+// CHECK-NEXT: %5 = load i32, ptr %.atomictmp2, align 4,              !dbg [[LINE25_G5R2:!.*]]
+// CHECK-NEXT: store atomic i32 %5, ptr @x monotonic, align 4,        !dbg [[LINE25_G5R1:!.*]]
+
+  int r3 = __atomic_test_and_set(&x, __ATOMIC_RELAXED);
+// CHECK-NEXT: %6 = atomicrmw xchg ptr @x, i8 1 monotonic, align 4,   !dbg [[LINE30:!.*]]
+// CHECK-NEXT: %tobool = icmp ne i8 %6, 0,                            !dbg [[LINE30_G7R2:!.*]]
+// CHECK-NEXT: store i1 %tobool, ptr %atomic-temp3, align 1,          !dbg [[LINE30_G7R1:!.*]]
+// CHECK-NEXT: %7 = load i8, ptr %atomic-temp3, align 1,              !dbg [[LINE30_G6R4:!.*]]
+// CHECK-NEXT: %loadedv = trunc i8 %7 to i1,                          !dbg [[LINE30_G6R3:!.*]]
+// CHECK-NEXT: %conv = zext i1 %loadedv to i32,                       !dbg [[LINE30_G6R2:!.*]]
+// CHECK-NEXT: store i32 %conv, ptr %r3, align 4,                     !dbg [[LINE30_G6R1:!.*]]
+
+  __atomic_clear(&x, __ATOMIC_RELAXED);
+// CHECK-NEXT: store atomic i8 0, ptr @x monotonic, align 4,          !dbg [[LINE39_G8R1:!.*]]
+
+  int r4 = __c11_atomic_exchange(&x, 2,__ATOMIC_RELAXED);
+// CHECK-NEXT: store i32 2, ptr %.atomictmp4, align 4,                !dbg [[LINE42_G10R1:!.*]]
+// CHECK-NEXT: %8 = load i32, ptr %.atomictmp4, align 4,              !dbg [[LINE42:!.*]]
+// CHECK-NEXT: %9 = atomicrmw xchg ptr @x, i32 %8 monotonic, align 4, !dbg [[LINE42_G10R2:!.*]]
+// CHECK-NEXT: store i32 %9, ptr %atomic-temp5, align 4,              !dbg [[LINE42_G10R1:!.*]]
+// CHECK-NEXT: %10 = load i32, ptr %atomic-temp5, align 4,            !dbg [[LINE42_G9R2:!.*]]
+// CHECK-NEXT: store i32 %10, ptr %r4, align 4,                       !dbg [[LINE42_G9R1:!.*]]
+
+  int r5 = __atomic_compare_exchange(&y, &y, &y, 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
+// CHECK-NEXT: %11 = load i32, ptr @y, align 4,                       !dbg [[LINE50:!.*]]
+// CHECK-NEXT: %12 = load i32, ptr @y, align 4,                       !dbg [[LINE50]]
+// CHECK-NEXT: %13 = cmpxchg ptr @y, i32 %11, i32 %12 monotonic monotonic, align 4, !dbg [[LINE50]]
+// CHECK-NEXT: %14 = extractvalue { i32, i1 } %13, 0,                 !dbg [[LINE50_G12R2:!.*]]
+// CHECK-NEXT: %15 = extractvalue { i32, i1 } %13, 1,                 !dbg [[LINE50_G12R3:!.*]]
+// CHECK-NEXT: br i1 %15, label %cmpxchg.continue, label %cmpxchg.store_expected, !dbg [[LINE50]]
+// CHECK: cmpxchg.store_expected:
+// CHECK-NEXT: store i32 %14, ptr @y, align 4,                        !dbg [[LINE50_G12R1:!.*]]
+// CHECK-NEXT: br label %cmpxchg.continue,                            !dbg [[LINE50]]
+// CHECK: cmpxchg.continue:
+// CHECK-NEXT: %storedv = zext i1 %15 to i8,                          !dbg [[LINE50_G12R2]]
+// CHECK-NEXT: store i8 %storedv, ptr %cmpxchg.bool, align 1,         !dbg [[LINE50_G12R1]]
+// CHECK-NEXT: %16 = load i8, ptr %cmpxchg.bool, align 1,             !dbg [[LINE50_G11R4:!.*]]
+// CHECK-NEXT: %loadedv6 = trunc i8 %16 to i1,                        !dbg [[LINE50_G11R3:!.*]]
+// CHECK-NEXT: %conv7 = zext i1 %loadedv6 to i32,                     !dbg [[LINE50_G11R2:!.*]]
+// CHECK-NEXT: store i32 %conv7, ptr %r5, align 4,                    !dbg [[LINE50_G11R1:!.*]]
+
+  int r6 = __c11_atomic_compare_exchange_strong(&x, &y, 42, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
+// CHECK-NEXT: store i32 42, ptr %.atomictmp8, align 4,               !dbg [[LINE68_G14R1:!.*]]
+// CHECK-NEXT: %17 = load i32, ptr @y, align 4,                       !dbg [[LINE68:!.*]]
+// CHECK-NEXT: %18 = load i32, ptr %.atomictmp8, align 4,             !dbg [[LINE68]]
+// CHECK-NEXT: %19 = cmpxchg ptr @x, i32 %17, i32 %18 monotonic monotonic, align 4, !dbg [[LINE68]]
+// CHECK-NEXT: %20 = extractvalue { i32, i1 } %19, 0,                 !dbg [[LINE68_G14R2:!.*]]
+// CHECK-NEXT: %21 = extractvalue { i32, i1 } %19, 1,                 !dbg [[LINE68_G14R3:!.*]]
+// CHECK-NEXT: br i1 %21, label %cmpxchg.continue11, label %cmpxchg.store_expected10, !dbg [[LINE68]]
+// CHECK: cmpxchg.store_expected10:
+// CHECK-NEXT: store i32 %20, ptr @y, align 4,                        !dbg [[LINE68_G14R1:!.*]]
+// CHECK-NEXT: br label %cmpxchg.continue11,                          !dbg [[LINE68]]
+// CHECK: cmpxchg.continue11:
+// CHECK-NEXT: %storedv12 = zext i1 %21 to i8,                        !dbg [[LINE68_G14R2]]
+// CHECK-NEXT: store i8 %storedv12, ptr %cmpxchg.bool9, align 1,      !dbg [[LINE68_G14R1:!.*]]
+// CHECK-NEXT: %22 = load i8, ptr %cmpxchg.bool9, align 1,            !dbg [[LINE68_G13R4:!.*]]
+// CHECK-NEXT: %loadedv13 = trunc i8 %22 to i1,                       !dbg [[LINE68_G13R3:!.*]]
+// CHECK-NEXT: %conv14 = zext i1 %loadedv13 to i32,                   !dbg [[LINE68_G13R2:!.*]]
+// CHECK-NEXT: store i32 %conv14, ptr %r6, align 4,                   !dbg [[LINE68_G13R1:!.*]]
+
+  int r7 = __c11_atomic_compare_exchange_weak(&x, &y, 43, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
+// CHECK-NEXT: store i32 43, ptr %.atomictmp15, align 4,              !dbg [[LINE87_G16R1:!.*]]
+// CHECK-NEXT: %23 = load i32, ptr @y, align 4,                       !dbg [[LINE87:!.*]]
+// CHECK-NEXT: %24 = load i32, ptr %.atomictmp15, align 4,            !dbg [[LINE87]]
+// CHECK-NEXT: %25 = cmpxchg weak ptr @x, i32 %23, i32 %24 monotonic monotonic, align 4, !dbg [[LINE87]]
+// CHECK-NEXT: %26 = extractvalue { i32, i1 } %25, 0,                 !dbg [[LINE87_G16R2:!.*]]
+// CHECK-NEXT: %27 = extractvalue { i32, i1 } %25, 1,                 !dbg [[LINE87_G16R3:!.*]]
+// CHECK-NEXT: br i1 %27, label %cmpxchg.continue18, label %cmpxchg.store_expected17, !dbg [[LINE87]]
+// CHECK: cmpxchg.store_expected17:
+// CHECK-NEXT: store i32 %26, ptr @y, align 4,                        !dbg [[LINE87_G16R1]]
+// CHECK-NEXT: br label %cmpxchg.continue18,                          !dbg [[LINE87]]
+// CHECK: cmpxchg.continue18:
+// CHECK-NEXT: %storedv19 = zext i1 %27 to i8,                        !dbg [[LINE87_G16R2]]
+// CHECK-NEXT: store i8 %storedv19, ptr %cmpxchg.bool16, align 1,     !dbg [[LINE87_G16R1]]
+// CHECK-NEXT: %28 = load i8, ptr %cmpxchg.bool16, align 1,           !dbg [[LINE87_G15R4:!.*]]
+// CHECK-NEXT: %loadedv20 = trunc i8 %28 to i1,                       !dbg [[LINE87_G15R3:!.*]]
+// CHECK-NEXT: %conv21 = zext i1 %loadedv20 to i32,                   !dbg [[LINE87_G15R2:!.*]]
+// CHECK-NEXT: store i32 %conv21, ptr %r7, align 4,                   !dbg [[LINE87_G15R1:!.*]]
+}
+
+// CHECK: [[LINE11_G2R1]] = !DILocation(line: 11, scope: ![[#]], atomGroup: 2, atomRank: 1)
+// CHECK: [[LINE11]]      = !DILocation(line: 11, scope: ![[#]])
+// CHECK: [[LINE11_G2R2]] = !DILocation(line: 11, scope: ![[#]], atomGroup: 2, atomRank: 2)
+// CHECK: [[LINE11_G1R2]] = !DILocation(line: 11, scope: ![[#]], atomGroup: 1, atomRank: 2)
+// CHECK: [[LINE11_G1R1]] = !DILocation(line: 11, scope: ![[#]], atomGroup: 1, atomRank: 1)
+
+// CHECK: [[LINE19_G4R2]] = !DILocation(line: 19, scope: ![[#]], atomGroup: 4, atomRank: 2)
+// CHECK: [[LINE19_G4R1]] = !DILocation(line: 19, scope: ![[#]], atomGroup: 4, atomRank: 1)
+// CHECK: [[LINE19_G3R2]] = !DILocation(line: 19, scope: ![[#]], atomGroup: 3, atomRank: 2)
+// CHECK: [[LINE19_G3R1]] = !DILocation(line: 19, scope: ![[#]], atomGroup: 3, atomRank: 1)
+
+// CHECK: [[LINE25_G5R1]] = !DILocation(line: 25, scope: ![[#]], atomGroup: 5, atomRank: 1)
+// CHECK: [[LINE25_G5R2]] = !DILocation(line: 25, scope: ![[#]], atomGroup: 5, atomRank: 2)
+
+// CHECK: [[LINE30]]      = !DILocation(line: 30, scope: ![[#]])
+// CHECK: [[LINE30_G7R2]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 7, atomRank: 2)
+// CHECK: [[LINE30_G7R1]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 7, atomRank: 1)
+// CHECK: [[LINE30_G6R4]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 6, atomRank: 4)
+// CHECK: [[LINE30_G6R3]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 6, atomRank: 3)
+// CHECK: [[LINE30_G6R2]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 6, atomRank: 2)
+// CHECK: [[LINE30_G6R1]] = !DILocation(line: 30, scope: ![[#]], atomGroup: 6, atomRank: 1)
+
+// CHECK: [[LINE39_G8R1]] = !DILocation(line: 39, scope: ![[#]], atomGroup: 8, atomRank: 1)
+
+// CHECK: [[LINE42_G10R1]] = !DILocation(line: 42, scope: ![[#]], atomGroup: 10, atomRank: 1)
+// CHECK: [[LINE42]]       = !DILocation(line: 42, scope: ![[#]])
+// CHECK: [[LINE42_G10R2]] = !DILocation(line: 42, scope: ![[#]], atomGroup: 10, atomRank: 2)
+// CHECK: [[LINE42_G9R2]]  = !DILocation(line: 42, scope: ![[#]], atomGroup: 9, atomRank: 2)
+// CHECK: [[LINE42_G9R1]]  = !DILocation(line: 42, scope: ![[#]], atomGroup: 9, atomRank: 1)
+
+// CHECK: [[LINE50]]       = !DILocation(line: 50, scope: ![[#]])
+// CHECK: [[LINE50_G12R2]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 12, atomRank: 2)
+// CHECK: [[LINE50_G12R3]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 12, atomRank: 3)
+// CHECK: [[LINE50_G12R1]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 12, atomRank: 1)
+// CHECK: [[LINE50_G11R4]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 11, atomRank: 4)
+// CHECK: [[LINE50_G11R3]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 11, atomRank: 3)
+// CHECK: [[LINE50_G11R2]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 11, atomRank: 2)
+// CHECK: [[LINE50_G11R1]] = !DILocation(line: 50, scope: ![[#]], atomGroup: 11, atomRank: 1)
+
+// CHECK: [[LINE68_G14R1]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 14, atomRank: 1)
+// CHECK: [[LINE68]]       = !DILocation(line: 68, scope: ![[#]])
+// CHECK: [[LINE68_G14R2]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 14, atomRank: 2)
+// CHECK: [[LINE68_G14R3]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 14, atomRank: 3)
+// CHECK: [[LINE68_G13R4]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 13, atomRank: 4)
+// CHECK: [[LINE68_G13R3]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 13, atomRank: 3)
+// CHECK: [[LINE68_G13R2]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 13, atomRank: 2)
+// CHECK: [[LINE68_G13R1]] = !DILocation(line: 68, scope: ![[#]], atomGroup: 13, atomRank: 1)
+
+// CHECK: [[LINE87_G16R1]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 16, atomRank: 1)
+// CHECK: [[LINE87]]       = !DILocation(line: 87, scope: ![[#]])
+// CHECK: [[LINE87_G16R2]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 16, atomRank: 2)
+// CHECK: [[LINE87_G16R3]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 16, atomRank: 3)
+// CHECK: [[LINE87_G15R4]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 15, atomRank: 4)
+// CHECK: [[LINE87_G15R3]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 15, atomRank: 3)
+// CHECK: [[LINE87_G15R2]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 15, atomRank: 2)
+// CHECK: [[LINE87_G15R1]] = !DILocation(line: 87, scope: ![[#]], atomGroup: 15, atomRank: 1)

@OCHyams
Copy link
Contributor Author

OCHyams commented Jun 10, 2025

Ping

Copy link
Member

@jmorse jmorse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with a test question

// CHECK: store i32 -1, ptr %.atomictmp, align 4, !dbg [[LINE11_G2R1:!.*]]
// CHECK-NEXT: %0 = load i32, ptr %.atomictmp, align 4, !dbg [[LINE11:!.*]]
// CHECK-NEXT: %1 = atomicrmw add ptr @x, i32 %0 monotonic, align 4, !dbg [[LINE11_G2R2:!.*]]
// CHECK-NEXT: store i32 %1, ptr %atomic-temp, align 4, !dbg [[LINE11_G2R1]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it's deliberate + desirable that both this and the earlier store of -1 are group-two-rank-one? (I suppose the buoy-based KIs will float the breakpoint upwards)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's deliberate. Possibly could be argued that the first store is "setup" and doesn't need to be key at all. I don't have a strong opinion either way, both should produce identical output if the instructions remain grouped like this (as you point out, due to the Buoys). The difference is if they split up over multiple blocks; the current impl has you stepping on the line twice, whereas alternative impls may have you step on this one.

Do you have a strong opinion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've no strong opinion; I suspect due to the stricter rules on atomic regions we wouldn't see this split over multiple blocks, but I wouldn't want to be quoted on that.

@OCHyams OCHyams merged commit ddecfa6 into llvm:main Jun 24, 2025
14 checks passed
OCHyams added a commit that referenced this pull request Jun 24, 2025
DrSergei pushed a commit to DrSergei/llvm-project that referenced this pull request Jun 24, 2025
This patch is part of a stack that teaches Clang to generate Key
Instructions metadata for C and C++.

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

RFC:
https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668
DrSergei pushed a commit to DrSergei/llvm-project that referenced this pull request Jun 24, 2025
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
This patch is part of a stack that teaches Clang to generate Key
Instructions metadata for C and C++.

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

RFC:
https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants