Skip to content

Commit 10f161b

Browse files
committed
[KeyInstr][Clang] Ret atom
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
1 parent 5408901 commit 10f161b

File tree

6 files changed

+147
-5
lines changed

6 files changed

+147
-5
lines changed

clang/lib/CodeGen/CGCall.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3883,7 +3883,8 @@ void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo &FI,
38833883

38843884
// Functions with no result always return void.
38853885
if (!ReturnValue.isValid()) {
3886-
Builder.CreateRetVoid();
3886+
auto *I = Builder.CreateRetVoid();
3887+
addRetToOverrideOrNewSourceAtom(I, nullptr);
38873888
return;
38883889
}
38893890

@@ -4065,6 +4066,9 @@ void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo &FI,
40654066

40664067
if (RetDbgLoc)
40674068
Ret->setDebugLoc(std::move(RetDbgLoc));
4069+
4070+
llvm::Value *Backup = RV ? Ret->getOperand(0) : nullptr;
4071+
addRetToOverrideOrNewSourceAtom(cast<llvm::ReturnInst>(Ret), Backup);
40684072
}
40694073

40704074
void CodeGenFunction::EmitReturnValueCheck(llvm::Value *RV) {

clang/lib/CodeGen/CGCleanup.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,8 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) {
11181118

11191119
// Create the branch.
11201120
llvm::BranchInst *BI = Builder.CreateBr(Dest.getBlock());
1121+
// This is the primary instruction for this atom, acting in place of a ret.
1122+
addInstToCurrentSourceAtom(BI, nullptr);
11211123

11221124
// Calculate the innermost active normal cleanup.
11231125
EHScopeStack::stable_iterator

clang/lib/CodeGen/CGStmt.cpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1594,6 +1594,7 @@ static bool isSwiftAsyncCallee(const CallExpr *CE) {
15941594
/// if the function returns void, or may be missing one if the function returns
15951595
/// non-void. Fun stuff :).
15961596
void CodeGenFunction::EmitReturnStmt(const ReturnStmt &S) {
1597+
ApplyAtomGroup Grp(getDebugInfo());
15971598
if (requiresReturnValueCheck()) {
15981599
llvm::Constant *SLoc = EmitCheckSourceLocation(S.getBeginLoc());
15991600
auto *SLocPtr =
@@ -1603,6 +1604,7 @@ void CodeGenFunction::EmitReturnStmt(const ReturnStmt &S) {
16031604
CGM.getSanitizerMetadata()->disableSanitizerForGlobal(SLocPtr);
16041605
assert(ReturnLocation.isValid() && "No valid return location");
16051606
Builder.CreateStore(SLocPtr, ReturnLocation);
1607+
//*OCH?*//
16061608
}
16071609

16081610
// Returning from an outlined SEH helper is UB, and we already warn on it.
@@ -1669,16 +1671,19 @@ void CodeGenFunction::EmitReturnStmt(const ReturnStmt &S) {
16691671
// If this function returns a reference, take the address of the expression
16701672
// rather than the value.
16711673
RValue Result = EmitReferenceBindingToExpr(RV);
1672-
Builder.CreateStore(Result.getScalarVal(), ReturnValue);
1674+
auto *I = Builder.CreateStore(Result.getScalarVal(), ReturnValue);
1675+
addInstToCurrentSourceAtom(I, I->getValueOperand());
16731676
} else {
16741677
switch (getEvaluationKind(RV->getType())) {
16751678
case TEK_Scalar: {
16761679
llvm::Value *Ret = EmitScalarExpr(RV);
1677-
if (CurFnInfo->getReturnInfo().getKind() == ABIArgInfo::Indirect)
1680+
if (CurFnInfo->getReturnInfo().getKind() == ABIArgInfo::Indirect) {
16781681
EmitStoreOfScalar(Ret, MakeAddrLValue(ReturnValue, RV->getType()),
16791682
/*isInit*/ true);
1680-
else
1681-
Builder.CreateStore(Ret, ReturnValue);
1683+
} else {
1684+
auto *I = Builder.CreateStore(Ret, ReturnValue);
1685+
addInstToCurrentSourceAtom(I, I->getValueOperand());
1686+
}
16821687
break;
16831688
}
16841689
case TEK_Complex:

clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "clang/CodeGen/CGFunctionInfo.h"
3737
#include "clang/Frontend/FrontendDiagnostic.h"
3838
#include "llvm/ADT/ArrayRef.h"
39+
#include "llvm/ADT/ScopeExit.h"
3940
#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
4041
#include "llvm/IR/DataLayout.h"
4142
#include "llvm/IR/Dominators.h"
@@ -339,6 +340,15 @@ llvm::DebugLoc CodeGenFunction::EmitReturnBlock() {
339340
// later by the actual 'ret' instruction.
340341
llvm::DebugLoc Loc = BI->getDebugLoc();
341342
Builder.SetInsertPoint(BI->getParent());
343+
344+
// Key Instructions: If there's only one `ret` then we want to put the
345+
// instruction in the same source atom group as the store to the ret-value
346+
// alloca and unconditional `br` to the return block that we're about to
347+
// delete. It all comes from the same source (`return (value)`).
348+
if (auto *DI = getDebugInfo(); DI && BI->getDebugLoc())
349+
DI->setRetInstSourceAtomOverride(
350+
BI->getDebugLoc().get()->getAtomGroup());
351+
342352
BI->eraseFromParent();
343353
delete ReturnBlock.getBlock();
344354
ReturnBlock = JumpDest();
@@ -1543,6 +1553,12 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
15431553
Bypasses.Init(CGM, Body);
15441554
}
15451555

1556+
// Finalize function debug info on exit.
1557+
auto Cleanup = llvm::make_scope_exit([this] {
1558+
if (CGDebugInfo *DI = getDebugInfo())
1559+
DI->completeFunction();
1560+
});
1561+
15461562
// Emit the standard function prologue.
15471563
StartFunction(GD, ResTy, Fn, FnInfo, Args, Loc, BodyRange.getBegin());
15481564

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// RUN: %clang -gkey-instructions -gno-column-info -x c++ %s -gmlt -S -emit-llvm -o - \
2+
// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
3+
4+
// RUN: %clang -gkey-instructions -gno-column-info -x c %s -gmlt -S -emit-llvm -o - \
5+
// RUN: | FileCheck %s --implicit-check-not atomGroup --implicit-check-not atomRank
6+
7+
typedef struct {
8+
struct{} a;
9+
double b;
10+
} s1;
11+
12+
s1 f(int z, ...) {
13+
__builtin_va_list list;
14+
__builtin_va_start(list, z);
15+
// CHECK: vaarg.end:
16+
// CHECK-NEXT: %vaarg.addr = phi ptr
17+
// CHECK-NEXT: call void @llvm.memcpy{{.*}}, !dbg [[G1R1:!.*]]
18+
// CHECK-NEXT: {{.*}} = getelementptr{{.*}}
19+
// CHECK-NEXT: [[LOAD:%.*]] = load double{{.*}}, !dbg [[G1R2:!.*]]
20+
// CHECK-NEXT: ret double [[LOAD]], !dbg [[G1R1]]
21+
return __builtin_va_arg(list, s1);
22+
}
23+
24+
// CHECK: [[G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
25+
// CHECK: [[G1R2]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 2)

clang/test/KeyInstructions/return.c

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// RUN: %clang -gkey-instructions -gno-column-info -x c++ %s -gmlt -S -emit-llvm -o - \
2+
// RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-CXX
3+
4+
// RUN: %clang -gkey-instructions -gno-column-info -x c %s -gmlt -S -emit-llvm -o - \
5+
// RUN: | FileCheck %s
6+
7+
// Check the stores to `retval` allocas and branches to `return` block are in
8+
// the same atom group. They are both rank 1, which could in theory introduce
9+
// an extra step in some optimized code. This low risk currently feels an
10+
// acceptable for keeping the code a bit simpler (as opposed to adding
11+
// scaffolding to make the store rank 2).
12+
13+
// Also check that in the case of a single return (no control flow) the
14+
// return instruction inherits the atom group of the branch to the return
15+
// block when the blocks get folded togather.
16+
17+
#ifdef __cplusplus
18+
#define nomangle extern "C"
19+
#else
20+
#define nomangle
21+
#endif
22+
23+
int g;
24+
nomangle float a() {
25+
// CHECK: float @a()
26+
if (g)
27+
// CHECK: if.then:
28+
// CHECK-NEXT: %1 = load i32, ptr @g{{.*}}, !dbg [[G2R3:!.*]]
29+
// CHECK-NEXT: %conv = sitofp i32 %1 to float{{.*}}, !dbg [[G2R2:!.*]]
30+
// CHECK-NEXT: store float %conv, ptr %retval{{.*}}, !dbg [[G2R1:!.*]]
31+
// CHECK-NEXT: br label %return{{.*}}, !dbg [[G2R1]]
32+
return g;
33+
// CHECK: if.end:
34+
// CHECK-NEXT: store float 1.000000e+00, ptr %retval{{.*}}, !dbg [[G3R1:!.*]]
35+
// CHECK-NEXT: br label %return, !dbg [[G3R1]]
36+
37+
// CHECK: return:
38+
// CHECK-NEXT: %2 = load float, ptr %retval{{.*}}, !dbg [[G4R2:!.*]]
39+
// CHECK-NEXT: ret float %2{{.*}}, !dbg [[G4R1:!.*]]
40+
return 1;
41+
}
42+
43+
// CHECK: void @b()
44+
// CHECK: ret void{{.*}}, !dbg [[B_G1R1:!.*]]
45+
nomangle void b() { return; }
46+
47+
// CHECK: i32 @c()
48+
// CHECK: %add = add{{.*}}, !dbg [[C_G1R2:!.*]]
49+
// CHECK: ret i32 %add{{.*}}, !dbg [[C_G1R1:!.*]]
50+
nomangle int c() { return g + 1; }
51+
52+
// NOTE: (return) (g = 1) are two separate atoms.
53+
// CHECK: i32 @d()
54+
// CHECK: store{{.*}}, !dbg [[D_G2R1:!.*]]
55+
// CHECK: ret i32 1{{.*}}, !dbg [[D_G1R1:!.*]]
56+
nomangle int d() { return g = 1; }
57+
58+
// The implicit return here get the line number of the closing brace; make it
59+
// key to match existing behaviour.
60+
// CHECK: ret void, !dbg [[E_G1R1:!.*]]
61+
nomangle void e() {}
62+
63+
#ifdef __cplusplus
64+
// CHECK-CXX: ptr @_Z1fRi
65+
int &f(int &r) {
66+
// Include ctrl-flow to stop ret value store being elided.
67+
if (r)
68+
// CHECK-CXX: if.then:
69+
// CHECK-CXX-NEXT: %2 = load ptr, ptr %r.addr{{.*}}, !dbg [[F_G2R2:!.*]]
70+
// CHECK-CXX-NEXT: store ptr %2, ptr %retval{{.*}}, !dbg [[F_G2R1:!.*]]
71+
// CHECK-CXX-NEXT: br label %return, !dbg [[F_G2R1:!.*]]
72+
return r;
73+
return g;
74+
}
75+
#endif
76+
77+
// CHECK: [[G2R3]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 3)
78+
// CHECK: [[G2R2]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 2)
79+
// CHECK: [[G2R1]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 1)
80+
// CHECK: [[G3R1]] = !DILocation({{.*}}, atomGroup: 3, atomRank: 1)
81+
// CHECK: [[G4R2]] = !DILocation({{.*}}, atomGroup: 4, atomRank: 2)
82+
// CHECK: [[G4R1]] = !DILocation({{.*}}, atomGroup: 4, atomRank: 1)
83+
// CHECK: [[B_G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
84+
// CHECK: [[C_G1R2]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 2)
85+
// CHECK: [[C_G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
86+
// CHECK: [[D_G2R1]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 1)
87+
// CHECK: [[D_G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
88+
// CHECK: [[E_G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
89+
// CHECK-CXX: [[F_G2R2]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 2)
90+
// CHECK-CXX: [[F_G2R1]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 1)

0 commit comments

Comments
 (0)