diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index d9fd406ad64e..65b71c39d86c 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 000000000000..3221ece69a71 --- /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)