-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[KeyInstr][Clang] For stmt atom #134646
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
base: users/OCHyams/ki-clang-while
Are you sure you want to change the base?
[KeyInstr][Clang] For stmt atom #134646
Conversation
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: #130943
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Orlando Cazalet-Hyams (OCHyams) ChangesThis patch is part of a stack that teaches Clang to generate Key Instructions The Key Instructions project is introduced, including a "quick summary" section The feature is only functional in LLVM if LLVM is built with CMake flag The Clang-side work is demoed here: Full diff: https://github.com/llvm/llvm-project/pull/134646.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index d9fd406ad64ee..65b71c39d86c4 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -1324,6 +1324,7 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
Continue = getJumpDestInCurrentScope("for.inc");
BreakContinueStack.push_back(BreakContinue(LoopExit, Continue));
+ llvm::BasicBlock *ForBody = nullptr;
if (S.getCond()) {
// If the for statement has a condition scope, emit the local variable
// declaration.
@@ -1348,7 +1349,7 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
ExitBlock = createBasicBlock("for.cond.cleanup");
// As long as the condition is true, iterate the loop.
- llvm::BasicBlock *ForBody = createBasicBlock("for.body");
+ ForBody = createBasicBlock("for.body");
// C99 6.8.5p2/p4: The first substatement is executed if the expression
// compares unequal to 0. The condition must be a scalar type.
@@ -1362,7 +1363,14 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
BoolCondVal = emitCondLikelihoodViaExpectIntrinsic(
BoolCondVal, Stmt::getLikelihood(S.getBody()));
- Builder.CreateCondBr(BoolCondVal, ForBody, ExitBlock, Weights);
+ auto *I = Builder.CreateCondBr(BoolCondVal, ForBody, ExitBlock, Weights);
+ // Key Instructions: Emit the condition and branch as separate atoms to
+ // match existing loop stepping behaviour. FIXME: We could have the branch
+ // as the backup location for the condition, which would probably be a
+ // better experience (no jumping to the brace).
+ if (auto *I = dyn_cast<llvm::Instruction>(BoolCondVal))
+ addInstToNewSourceAtom(I, nullptr);
+ addInstToNewSourceAtom(I, nullptr);
if (ExitBlock != LoopExit.getBlock()) {
EmitBlock(ExitBlock);
@@ -1416,6 +1424,12 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
if (CGM.shouldEmitConvergenceTokens())
ConvergenceTokenStack.pop_back();
+
+ if (ForBody) {
+ // Key Instructions: We want the for closing brace to be step-able on to
+ // match existing behaviour.
+ addInstToNewSourceAtom(ForBody->getTerminator(), nullptr);
+ }
}
void
diff --git a/clang/test/KeyInstructions/for.c b/clang/test/KeyInstructions/for.c
new file mode 100644
index 0000000000000..3221ece69a717
--- /dev/null
+++ b/clang/test/KeyInstructions/for.c
@@ -0,0 +1,37 @@
+// RUN: %clang -gkey-instructions -x c++ %s -gmlt -S -emit-llvm -o - \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+
+// RUN: %clang -gkey-instructions -x c %s -gmlt -S -emit-llvm -o - \
+// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
+
+// Perennial quesiton: should the inc be its own source atom or not
+// (currently it is).
+
+// FIXME: See do.c and while.c regarding cmp and cond br groups.
+
+void a(int A) {
+// CHECK: entry:
+// CHECK: store i32 0, ptr %i{{.*}}, !dbg [[G1R1:!.*]]
+// CHECK: for.cond:
+// CHECK: %cmp = icmp slt i32 %0, %1, !dbg [[G2R1:!.*]]
+// CHECK: br i1 %cmp, label %for.body, label %for.end, !dbg [[G3R1:!.*]]
+
+// FIXME: Added uncond br group here which is useful for O0, which we're
+// no longer targeting. With optimisations loop rotate puts the condition
+// into for.inc and simplifycfg smooshes that and for.body together, so
+// it's not clear whether it adds any value.
+// CHECK: for.body:
+// CHECK: br label %for.inc, !dbg [[G5R1:!.*]]
+
+// CHECK: for.inc:
+// CHECK: %inc = add{{.*}}, !dbg [[G4R2:!.*]]
+// CHECK: store i32 %inc, ptr %i{{.*}}, !dbg [[G4R1:!.*]]
+ for (int i = 0; i < A; ++i) { }
+}
+
+// CHECK: [[G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
+// CHECK: [[G2R1]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 1)
+// CHECK: [[G3R1]] = !DILocation({{.*}}, atomGroup: 3, atomRank: 1)
+// CHECK: [[G5R1]] = !DILocation({{.*}}, atomGroup: 5, atomRank: 1)
+// CHECK: [[G4R2]] = !DILocation({{.*}}, atomGroup: 4, atomRank: 2)
+// CHECK: [[G4R1]] = !DILocation({{.*}}, atomGroup: 4, atomRank: 1)
|
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:
#130943