Skip to content

Commit a7c1aee

Browse files
authored
Merge pull request #585 from vedantk/cherry
Cherry-pick follow-ups to D72489 (AT_call_return_pc DWARF emission change)
2 parents be9f5e1 + b2c99bc commit a7c1aee

File tree

11 files changed

+326
-27
lines changed

11 files changed

+326
-27
lines changed

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def setUp(self):
1717

1818
@skipIf(compiler="clang", compiler_version=['<', '8.0'])
1919
@skipIf(dwarf_version=['<', '4'])
20-
@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
20+
@skipUnlessDarwin # llvm.org/PR44561
2121
def test_cross_dso_tail_calls(self):
2222
self.build()
2323
exe = self.getBuildArtifact("a.out")

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def setUp(self):
1717

1818
@skipIf(compiler="clang", compiler_version=['<', '8.0'])
1919
@skipIf(dwarf_version=['<', '4'])
20-
@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
20+
@skipUnlessDarwin # llvm.org/PR44561
2121
def test_cross_object_tail_calls(self):
2222
self.build()
2323
exe = self.getBuildArtifact("a.out")

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,7 @@ DwarfCompileUnit::getDwarf5OrGNUAttr(dwarf::Attribute Attr) const {
934934
case dwarf::DW_AT_call_origin:
935935
return dwarf::DW_AT_abstract_origin;
936936
case dwarf::DW_AT_call_pc:
937+
case dwarf::DW_AT_call_return_pc:
937938
return dwarf::DW_AT_low_pc;
938939
case dwarf::DW_AT_call_value:
939940
return dwarf::DW_AT_GNU_call_site_value;
@@ -982,12 +983,13 @@ DIE &DwarfCompileUnit::constructCallSiteEntryDIE(DIE &ScopeDIE,
982983

983984
// Attach the return PC to allow the debugger to disambiguate call paths
984985
// from one function to another.
985-
if (DD->getDwarfVersion() == 4 && DD->tuneForGDB()) {
986-
assert(PCAddr && "Missing PC information for a call");
987-
addLabelAddress(CallSiteDIE, dwarf::DW_AT_low_pc, PCAddr);
988-
} else if (!IsTail || DD->tuneForGDB()) {
986+
//
987+
// The return PC is only really needed when the call /isn't/ a tail call, but
988+
// for some reason GDB always expects it.
989+
if (!IsTail || DD->tuneForGDB()) {
989990
assert(PCAddr && "Missing return PC information for a call");
990-
addLabelAddress(CallSiteDIE, dwarf::DW_AT_call_return_pc, PCAddr);
991+
addLabelAddress(CallSiteDIE,
992+
getDwarf5OrGNUAttr(dwarf::DW_AT_call_return_pc), PCAddr);
991993
}
992994

993995
return CallSiteDIE;

llvm/lib/Transforms/Utils/CodeExtractor.cpp

Lines changed: 127 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,14 @@
3131
#include "llvm/IR/CFG.h"
3232
#include "llvm/IR/Constant.h"
3333
#include "llvm/IR/Constants.h"
34+
#include "llvm/IR/DIBuilder.h"
3435
#include "llvm/IR/DataLayout.h"
36+
#include "llvm/IR/DebugInfoMetadata.h"
3537
#include "llvm/IR/DerivedTypes.h"
3638
#include "llvm/IR/Dominators.h"
3739
#include "llvm/IR/Function.h"
3840
#include "llvm/IR/GlobalValue.h"
41+
#include "llvm/IR/InstIterator.h"
3942
#include "llvm/IR/InstrTypes.h"
4043
#include "llvm/IR/Instruction.h"
4144
#include "llvm/IR/Instructions.h"
@@ -1382,6 +1385,129 @@ void CodeExtractor::calculateNewCallTerminatorWeights(
13821385
MDBuilder(TI->getContext()).createBranchWeights(BranchWeights));
13831386
}
13841387

1388+
/// Erase debug info intrinsics which refer to values in \p F but aren't in
1389+
/// \p F.
1390+
static void eraseDebugIntrinsicsWithNonLocalRefs(Function &F) {
1391+
for (Instruction &I : instructions(F)) {
1392+
SmallVector<DbgVariableIntrinsic *, 4> DbgUsers;
1393+
findDbgUsers(DbgUsers, &I);
1394+
for (DbgVariableIntrinsic *DVI : DbgUsers)
1395+
if (DVI->getFunction() != &F)
1396+
DVI->eraseFromParent();
1397+
}
1398+
}
1399+
1400+
/// Fix up the debug info in the old and new functions by pointing line
1401+
/// locations and debug intrinsics to the new subprogram scope, and by deleting
1402+
/// intrinsics which point to values outside of the new function.
1403+
static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc,
1404+
CallInst &TheCall) {
1405+
DISubprogram *OldSP = OldFunc.getSubprogram();
1406+
LLVMContext &Ctx = OldFunc.getContext();
1407+
1408+
// See llvm.org/PR44560, OpenMP passes an invalid subprogram to CodeExtractor.
1409+
bool NeedWorkaroundForOpenMPIRBuilderBug =
1410+
OldSP && OldSP->getRetainedNodes()->isTemporary();
1411+
1412+
if (!OldSP || NeedWorkaroundForOpenMPIRBuilderBug) {
1413+
// Erase any debug info the new function contains.
1414+
stripDebugInfo(NewFunc);
1415+
// Make sure the old function doesn't contain any non-local metadata refs.
1416+
eraseDebugIntrinsicsWithNonLocalRefs(NewFunc);
1417+
return;
1418+
}
1419+
1420+
// Create a subprogram for the new function. Leave out a description of the
1421+
// function arguments, as the parameters don't correspond to anything at the
1422+
// source level.
1423+
assert(OldSP->getUnit() && "Missing compile unit for subprogram");
1424+
DIBuilder DIB(*OldFunc.getParent(), /*AllowUnresolvedNodes=*/false,
1425+
OldSP->getUnit());
1426+
auto SPType = DIB.createSubroutineType(DIB.getOrCreateTypeArray(None));
1427+
DISubprogram::DISPFlags SPFlags = DISubprogram::SPFlagDefinition |
1428+
DISubprogram::SPFlagOptimized |
1429+
DISubprogram::SPFlagLocalToUnit;
1430+
auto NewSP = DIB.createFunction(
1431+
OldSP->getUnit(), NewFunc.getName(), NewFunc.getName(), OldSP->getFile(),
1432+
/*LineNo=*/0, SPType, /*ScopeLine=*/0, DINode::FlagZero, SPFlags);
1433+
NewFunc.setSubprogram(NewSP);
1434+
1435+
// Debug intrinsics in the new function need to be updated in one of two
1436+
// ways:
1437+
// 1) They need to be deleted, because they describe a value in the old
1438+
// function.
1439+
// 2) They need to point to fresh metadata, e.g. because they currently
1440+
// point to a variable in the wrong scope.
1441+
SmallDenseMap<DINode *, DINode *> RemappedMetadata;
1442+
SmallVector<Instruction *, 4> DebugIntrinsicsToDelete;
1443+
for (Instruction &I : instructions(NewFunc)) {
1444+
auto *DII = dyn_cast<DbgInfoIntrinsic>(&I);
1445+
if (!DII)
1446+
continue;
1447+
1448+
// Point the intrinsic to a fresh label within the new function.
1449+
if (auto *DLI = dyn_cast<DbgLabelInst>(&I)) {
1450+
DILabel *OldLabel = DLI->getLabel();
1451+
DINode *&NewLabel = RemappedMetadata[OldLabel];
1452+
if (!NewLabel)
1453+
NewLabel = DILabel::get(Ctx, NewSP, OldLabel->getName(),
1454+
OldLabel->getFile(), OldLabel->getLine());
1455+
DLI->setArgOperand(0, MetadataAsValue::get(Ctx, NewLabel));
1456+
continue;
1457+
}
1458+
1459+
// If the location isn't a constant or an instruction, delete the
1460+
// intrinsic.
1461+
auto *DVI = cast<DbgVariableIntrinsic>(DII);
1462+
Value *Location = DVI->getVariableLocation();
1463+
if (!Location ||
1464+
(!isa<Constant>(Location) && !isa<Instruction>(Location))) {
1465+
DebugIntrinsicsToDelete.push_back(DVI);
1466+
continue;
1467+
}
1468+
1469+
// If the variable location is an instruction but isn't in the new
1470+
// function, delete the intrinsic.
1471+
Instruction *LocationInst = dyn_cast<Instruction>(Location);
1472+
if (LocationInst && LocationInst->getFunction() != &NewFunc) {
1473+
DebugIntrinsicsToDelete.push_back(DVI);
1474+
continue;
1475+
}
1476+
1477+
// Point the intrinsic to a fresh variable within the new function.
1478+
DILocalVariable *OldVar = DVI->getVariable();
1479+
DINode *&NewVar = RemappedMetadata[OldVar];
1480+
if (!NewVar)
1481+
NewVar = DIB.createAutoVariable(
1482+
NewSP, OldVar->getName(), OldVar->getFile(), OldVar->getLine(),
1483+
OldVar->getType(), /*AlwaysPreserve=*/false, DINode::FlagZero,
1484+
OldVar->getAlignInBits());
1485+
DVI->setArgOperand(1, MetadataAsValue::get(Ctx, NewVar));
1486+
}
1487+
for (auto *DII : DebugIntrinsicsToDelete)
1488+
DII->eraseFromParent();
1489+
DIB.finalizeSubprogram(NewSP);
1490+
1491+
// Fix up the scope information attached to the line locations in the new
1492+
// function.
1493+
for (Instruction &I : instructions(NewFunc)) {
1494+
if (const DebugLoc &DL = I.getDebugLoc())
1495+
I.setDebugLoc(DebugLoc::get(DL.getLine(), DL.getCol(), NewSP));
1496+
1497+
// Loop info metadata may contain line locations. Fix them up.
1498+
auto updateLoopInfoLoc = [&Ctx,
1499+
NewSP](const DILocation &Loc) -> DILocation * {
1500+
return DILocation::get(Ctx, Loc.getLine(), Loc.getColumn(), NewSP,
1501+
nullptr);
1502+
};
1503+
updateLoopMetadataDebugLocations(I, updateLoopInfoLoc);
1504+
}
1505+
if (!TheCall.getDebugLoc())
1506+
TheCall.setDebugLoc(DebugLoc::get(0, 0, OldSP));
1507+
1508+
eraseDebugIntrinsicsWithNonLocalRefs(NewFunc);
1509+
}
1510+
13851511
Function *
13861512
CodeExtractor::extractCodeRegion(const CodeExtractorAnalysisCache &CEAC) {
13871513
if (!isEligible())
@@ -1567,26 +1693,7 @@ CodeExtractor::extractCodeRegion(const CodeExtractorAnalysisCache &CEAC) {
15671693
}
15681694
}
15691695

1570-
// Erase debug info intrinsics. Variable updates within the new function are
1571-
// invisible to debuggers. This could be improved by defining a DISubprogram
1572-
// for the new function.
1573-
for (BasicBlock &BB : *newFunction) {
1574-
auto BlockIt = BB.begin();
1575-
// Remove debug info intrinsics from the new function.
1576-
while (BlockIt != BB.end()) {
1577-
Instruction *Inst = &*BlockIt;
1578-
++BlockIt;
1579-
if (isa<DbgInfoIntrinsic>(Inst))
1580-
Inst->eraseFromParent();
1581-
}
1582-
// Remove debug info intrinsics which refer to values in the new function
1583-
// from the old function.
1584-
SmallVector<DbgVariableIntrinsic *, 4> DbgUsers;
1585-
for (Instruction &I : BB)
1586-
findDbgUsers(DbgUsers, &I);
1587-
for (DbgVariableIntrinsic *DVI : DbgUsers)
1588-
DVI->eraseFromParent();
1589-
}
1696+
fixupDebugInfoPostExtraction(*oldFunction, *newFunction, *TheCall);
15901697

15911698
// Mark the new function `noreturn` if applicable. Terminators which resume
15921699
// exception propagation are treated as returning instructions. This is to
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
; RUN: opt -hotcoldsplit -hotcoldsplit-threshold=0 -S < %s | FileCheck %s
2+
3+
; When an llvm.dbg.label intrinsic is extracted into a new function, make sure
4+
; that its metadata argument is a DILabel that points to a scope within the new
5+
; function.
6+
;
7+
; In this example, the label "bye" points to the scope for @foo before
8+
; splitting, and should point to the scope for @foo.cold.1 after splitting.
9+
10+
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
11+
target triple = "x86_64-apple-macosx10.14.0"
12+
13+
; CHECK-LABEL: define {{.*}}@foo.cold.1
14+
; CHECK: llvm.dbg.label(metadata [[LABEL:![0-9]+]]), !dbg [[LINE:![0-9]+]]
15+
16+
; CHECK: [[FILE:![0-9]+]] = !DIFile
17+
; CHECK: [[SCOPE:![0-9]+]] = distinct !DISubprogram(name: "foo.cold.1"
18+
; CHECK: [[LINE]] = !DILocation(line: 1, column: 1, scope: [[SCOPE]]
19+
; CHECK: [[LABEL]] = !DILabel(scope: [[SCOPE]], name: "bye", file: [[FILE]], line: 28
20+
21+
define void @foo(i32 %arg1) !dbg !6 {
22+
entry:
23+
%var = add i32 0, 0, !dbg !11
24+
br i1 undef, label %if.then, label %if.end
25+
26+
if.then: ; preds = %entry
27+
ret void
28+
29+
if.end: ; preds = %entry
30+
call void @llvm.dbg.label(metadata !12), !dbg !11
31+
call void @sink()
32+
ret void
33+
}
34+
35+
declare void @llvm.dbg.label(metadata)
36+
37+
declare void @sink() cold
38+
39+
!llvm.dbg.cu = !{!0}
40+
!llvm.debugify = !{!3, !4}
41+
!llvm.module.flags = !{!5}
42+
43+
!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
44+
!1 = !DIFile(filename: "<stdin>", directory: "/")
45+
!2 = !{}
46+
!3 = !{i32 7}
47+
!4 = !{i32 1}
48+
!5 = !{i32 2, !"Debug Info Version", i32 3}
49+
!6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, unit: !0, retainedNodes: !8)
50+
!7 = !DISubroutineType(types: !2)
51+
!8 = !{!9}
52+
!9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
53+
!10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned)
54+
!11 = !DILocation(line: 1, column: 1, scope: !6)
55+
!12 = !DILabel(scope: !6, name: "bye", file: !1, line: 28)
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
; RUN: opt -hotcoldsplit -hotcoldsplit-threshold=0 -S < %s | FileCheck %s
2+
3+
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
4+
target triple = "x86_64-apple-macosx10.14.0"
5+
6+
; The block "if.end" in @foo is extracted into a new function, @foo.cold.1.
7+
; Check the following:
8+
9+
; CHECK-LABEL: define {{.*}}@foo.cold.1
10+
11+
; - The llvm.dbg.value intrinsic pointing to an argument in @foo (%arg1) is
12+
; dropped
13+
; CHECK-NOT: llvm.dbg.value
14+
15+
; - Instructions without locations in the original function have no
16+
; location in the new function
17+
; CHECK: [[ADD1:%.*]] = add i32 %{{.*}}, 1{{$}}
18+
19+
; - Ditto (see above), calls are not special
20+
; CHECK-NEXT: call void @sink(i32 [[ADD1]])
21+
22+
; - Line locations are preserved
23+
; CHECK-NEXT: call void @sink(i32 [[ADD1]]), !dbg [[LINE1:![0-9]+]]
24+
25+
; - llvm.dbg.value intrinsics for values local to @foo.cold.1 are preserved
26+
; CHECK-NEXT: llvm.dbg.value(metadata i32 [[ADD1]], metadata [[VAR1:![0-9]+]], metadata !DIExpression()), !dbg [[LINE1]]
27+
28+
; - Expressions inside of dbg.value intrinsics are preserved
29+
; CHECK-NEXT: llvm.dbg.value(metadata i32 [[ADD1]], metadata [[VAR1]], metadata !DIExpression(DW_OP_constu, 1, DW_OP_plus, DW_OP_stack_value)
30+
31+
; - The DISubprogram for @foo.cold.1 has an empty DISubroutineType
32+
; CHECK: [[FILE:![0-9]+]] = !DIFile(filename: "<stdin>"
33+
; CHECK: [[EMPTY_MD:![0-9]+]] = !{}
34+
; CHECK: [[EMPTY_TYPE:![0-9]+]] = !DISubroutineType(types: [[EMPTY_MD]])
35+
; CHECK: [[NEWSCOPE:![0-9]+]] = distinct !DISubprogram(name: "foo.cold.1", linkageName: "foo.cold.1", scope: null, file: [[FILE]], type: [[EMPTY_TYPE]], spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized
36+
37+
; - Line locations in @foo.cold.1 point to the new scope for @foo.cold.1
38+
; CHECK: [[LINE1]] = !DILocation(line: 1, column: 1, scope: [[NEWSCOPE]])
39+
40+
define void @foo(i32 %arg1) !dbg !6 {
41+
entry:
42+
%var = add i32 0, 0, !dbg !11
43+
br i1 undef, label %if.then, label %if.end
44+
45+
if.then: ; preds = %entry
46+
ret void
47+
48+
if.end: ; preds = %entry
49+
call void @llvm.dbg.value(metadata i32 %arg1, metadata !9, metadata !DIExpression()), !dbg !11
50+
%add1 = add i32 %arg1, 1
51+
call void @sink(i32 %add1)
52+
call void @sink(i32 %add1), !dbg !11
53+
call void @llvm.dbg.value(metadata i32 %add1, metadata !9, metadata !DIExpression()), !dbg !11
54+
call void @llvm.dbg.value(metadata i32 %add1, metadata !9, metadata !DIExpression(DW_OP_constu, 1, DW_OP_plus, DW_OP_stack_value)), !dbg !11
55+
ret void
56+
}
57+
58+
declare void @llvm.dbg.value(metadata, metadata, metadata)
59+
60+
declare void @sink(i32) cold
61+
62+
!llvm.dbg.cu = !{!0}
63+
!llvm.debugify = !{!3, !4}
64+
!llvm.module.flags = !{!5}
65+
66+
!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
67+
!1 = !DIFile(filename: "<stdin>", directory: "/")
68+
!2 = !{}
69+
!3 = !{i32 7}
70+
!4 = !{i32 1}
71+
!5 = !{i32 2, !"Debug Info Version", i32 3}
72+
!6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, unit: !0, retainedNodes: !8)
73+
!7 = !DISubroutineType(types: !2)
74+
!8 = !{!9}
75+
!9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
76+
!10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned)
77+
!11 = !DILocation(line: 1, column: 1, scope: !6)

0 commit comments

Comments
 (0)