Skip to content

Commit a83bfab

Browse files
authored
[DebugInfo] Stop relying on asm gadgets to extend variable ranges (#29029)
The 'fake use' asm gadget does not always keep variables alive. E.g., in rdar://57754659, llvm was unable to preserve two local variables despite the use of these gadgets. These variables were backed by a LoadInst and an ExtractValueInst respectively. Instead of emitting shadow copies for just those kinds of instructions, use shadow copies exclusively. This may cause more variables to appear in the debugger window before they are initialized, but should result in fewer variables being dropped. rdar://57754659
1 parent 8fe4c07 commit a83bfab

File tree

5 files changed

+37
-183
lines changed

5 files changed

+37
-183
lines changed

lib/IRGen/IRGenSIL.cpp

Lines changed: 6 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -389,10 +389,6 @@ class IRGenSILFunction :
389389
/// Keeps track of the mapping of source variables to -O0 shadow copy allocas.
390390
llvm::SmallDenseMap<StackSlotKey, Address, 8> ShadowStackSlots;
391391
llvm::SmallDenseMap<Decl *, SmallString<4>, 8> AnonymousVariables;
392-
/// To avoid inserting elements into ValueDomPoints twice.
393-
llvm::SmallDenseSet<llvm::Instruction *, 8> ValueVariables;
394-
/// Holds the DominancePoint of values that are storage for a source variable.
395-
SmallVector<std::pair<llvm::Instruction *, DominancePoint>, 8> ValueDomPoints;
396392
unsigned NumAnonVars = 0;
397393

398394
/// Accumulative amount of allocated bytes on the stack. Used to limit the
@@ -661,78 +657,6 @@ class IRGenSILFunction :
661657
return Name;
662658
}
663659

664-
/// Try to emit an inline assembly gadget which extends the lifetime of
665-
/// \p Var. Returns whether or not this was successful.
666-
bool emitLifetimeExtendingUse(llvm::Value *Var) {
667-
llvm::Type *ArgTys;
668-
auto *Ty = Var->getType();
669-
// Vectors, Pointers and Floats are expected to fit into a register.
670-
if (Ty->isPointerTy() || Ty->isFloatingPointTy() || Ty->isVectorTy())
671-
ArgTys = {Ty};
672-
else {
673-
// If this is not a scalar or vector type, we can't handle it.
674-
if (isa<llvm::CompositeType>(Ty))
675-
return false;
676-
// The storage is guaranteed to be no larger than the register width.
677-
// Extend the storage so it would fit into a register.
678-
llvm::Type *IntTy;
679-
switch (IGM.getClangASTContext().getTargetInfo().getRegisterWidth()) {
680-
case 64:
681-
IntTy = IGM.Int64Ty;
682-
break;
683-
case 32:
684-
IntTy = IGM.Int32Ty;
685-
break;
686-
default:
687-
llvm_unreachable("unsupported register width");
688-
}
689-
ArgTys = {IntTy};
690-
Var = Builder.CreateZExtOrBitCast(Var, IntTy);
691-
}
692-
// Emit an empty inline assembler expression depending on the register.
693-
auto *AsmFnTy = llvm::FunctionType::get(IGM.VoidTy, ArgTys, false);
694-
auto *InlineAsm = llvm::InlineAsm::get(AsmFnTy, "", "r", true);
695-
Builder.CreateAsmCall(InlineAsm, Var);
696-
return true;
697-
}
698-
699-
/// At -Onone, forcibly keep all LLVM values that are tracked by
700-
/// debug variables alive by inserting an empty inline assembler
701-
/// expression depending on the value in the blocks dominated by the
702-
/// value.
703-
void emitDebugVariableRangeExtension(const SILBasicBlock *CurBB) {
704-
if (IGM.IRGen.Opts.shouldOptimize())
705-
return;
706-
for (auto &Variable : ValueDomPoints) {
707-
llvm::Instruction *Var = Variable.first;
708-
DominancePoint VarDominancePoint = Variable.second;
709-
if (getActiveDominancePoint() == VarDominancePoint ||
710-
isActiveDominancePointDominatedBy(VarDominancePoint)) {
711-
bool ExtendedLifetime = emitLifetimeExtendingUse(Var);
712-
if (!ExtendedLifetime)
713-
continue;
714-
715-
// Propagate dbg.values for Var into the current basic block. Note
716-
// that this shouldn't be necessary. LiveDebugValues should be doing
717-
// this but can't in general because it currently only tracks register
718-
// locations.
719-
llvm::BasicBlock *BB = Var->getParent();
720-
llvm::BasicBlock *CurBB = Builder.GetInsertBlock();
721-
if (BB == CurBB)
722-
// The current basic block must be a successor of the dbg.value().
723-
continue;
724-
725-
llvm::SmallVector<llvm::DbgValueInst *, 4> DbgValues;
726-
llvm::findDbgValues(DbgValues, Var);
727-
for (auto *DVI : DbgValues)
728-
if (DVI->getParent() == BB)
729-
IGM.DebugInfo->getBuilder().insertDbgValueIntrinsic(
730-
DVI->getValue(), DVI->getVariable(), DVI->getExpression(),
731-
DVI->getDebugLoc(), &*CurBB->getFirstInsertionPt());
732-
}
733-
}
734-
}
735-
736660
/// To make it unambiguous whether a `var` binding has been initialized,
737661
/// zero-initialize the shadow copy alloca. LLDB uses the first pointer-sized
738662
/// field to recognize to detect uninitizialized variables. This can be
@@ -757,16 +681,17 @@ class IRGenSILFunction :
757681

758682
/// Account for bugs in LLVM.
759683
///
684+
/// - When a variable is spilled into a stack slot, LiveDebugValues fails to
685+
/// recognize a restore of that slot for a different variable.
686+
///
760687
/// - The LLVM type legalizer currently doesn't update debug
761688
/// intrinsics when a large value is split up into smaller
762689
/// pieces. Note that this heuristic as a bit too conservative
763690
/// on 32-bit targets as it will also fire for doubles.
764691
///
765692
/// - CodeGen Prepare may drop dbg.values pointing to PHI instruction.
766693
bool needsShadowCopy(llvm::Value *Storage) {
767-
return (IGM.DataLayout.getTypeSizeInBits(Storage->getType()) >
768-
IGM.getClangASTContext().getTargetInfo().getRegisterWidth()) ||
769-
isa<llvm::PHINode>(Storage);
694+
return !isa<llvm::Constant>(Storage);
770695
}
771696

772697
/// Unconditionally emit a stack shadow copy of an \c llvm::Value.
@@ -800,27 +725,10 @@ class IRGenSILFunction :
800725
if (IGM.IRGen.Opts.DisableDebuggerShadowCopies ||
801726
IGM.IRGen.Opts.shouldOptimize() || IsAnonymous ||
802727
isa<llvm::AllocaInst>(Storage) || isa<llvm::UndefValue>(Storage) ||
803-
Storage->getType() == IGM.RefCountedPtrTy)
728+
Storage->getType() == IGM.RefCountedPtrTy || !needsShadowCopy(Storage))
804729
return Storage;
805730

806-
// Always emit shadow copies for function arguments.
807-
if (VarInfo.ArgNo == 0)
808-
// Otherwise only if debug value range extension is not feasible.
809-
if (!needsShadowCopy(Storage)) {
810-
// Mark for debug value range extension unless this is a constant, or
811-
// unless it's not possible to emit lifetime-extending uses for this.
812-
if (auto *Value = dyn_cast<llvm::Instruction>(Storage)) {
813-
// Emit a use at the start of the storage lifetime to force early
814-
// materialization. This makes variables available for inspection as
815-
// soon as they are defined.
816-
bool ExtendedLifetime = emitLifetimeExtendingUse(Value);
817-
if (ExtendedLifetime)
818-
if (ValueVariables.insert(Value).second)
819-
ValueDomPoints.push_back({Value, getActiveDominancePoint()});
820-
}
821-
822-
return Storage;
823-
}
731+
// Emit a shadow copy.
824732
return emitShadowCopy(Storage, Scope, VarInfo, Align);
825733
}
826734

@@ -1900,9 +1808,6 @@ void IRGenSILFunction::visitSILBasicBlock(SILBasicBlock *BB) {
19001808
IGM.DebugInfo->setCurrentLoc(
19011809
Builder, DS, RegularLocation::getAutoGeneratedLocation());
19021810
}
1903-
1904-
if (isa<TermInst>(&I))
1905-
emitDebugVariableRangeExtension(BB);
19061811
}
19071812
visit(&I);
19081813
}

test/DebugInfo/liverange-extension-vector.swift

Lines changed: 0 additions & 15 deletions
This file was deleted.

test/DebugInfo/liverange-extension.swift

Lines changed: 0 additions & 63 deletions
This file was deleted.

test/DebugInfo/shadow_copies.swift

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,45 @@ class ClassB : ClassA
1818
override init (_ input : Int64)
1919
{
2020
// CHECK: @"$s{{.*}}6ClassBCyACs5Int64Vcfc"
21-
// NOPCOPY: @"$s{{.*}}6ClassBCyACs5Int64Vcfc"
21+
// NOCOPY: @"$s{{.*}}6ClassBCyACs5Int64Vcfc"
2222
// CHECK: alloca {{.*}}ClassBC*
23-
// NOPCOPY: alloca {{.*}}ClassBC*
23+
// NOCOPY: alloca {{.*}}ClassBC*
2424

2525
// CHECK: alloca i64
2626

2727
// CHECK-NOT: alloca
28-
// NOPCOPY-NOT: alloca
28+
// NOCOPY-NOT: alloca
2929
// CHECK: ret {{.*}}ClassBC
3030
// NOCOPY: ret {{.*}}ClassBC
3131
super.init (input)
3232
}
3333
}
3434

3535
let b = ClassB(1);
36+
37+
func use(_ x: Int) {}
38+
39+
class ClassC
40+
{
41+
// CHECK: define {{.*}}@"$s13shadow_copies6ClassCCACycfc"
42+
// NOCOPY: define {{.*}}@"$s13shadow_copies6ClassCCACycfc"
43+
init ()
44+
{
45+
// CHECK: alloca %T13shadow_copies6ClassCC*
46+
// CHECK-NOT: alloca
47+
// NOCOPY-NOT: alloca
48+
49+
// CHECK: call void @llvm.dbg.value(metadata i64 10
50+
// NOCOPY: call void @llvm.dbg.value(metadata i64 10
51+
let x = 10
52+
53+
use(x)
54+
55+
use(x)
56+
57+
// CHECK: ret
58+
// NOCOPY: ret
59+
}
60+
}
61+
62+
let c = ClassC()

test/IRGen/unmanaged_objc_throw_func.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import Foundation
1818
// CHECK-NEXT: store i{{32|64}} 1, i{{32|64}}* %._value, align {{[0-9]+}}
1919
// CHECK-NEXT: %[[T4:.+]] = call swiftcc %TSo7NSArrayC* @"$sSa10FoundationE19_bridgeToObjectiveCSo7NSArrayCyF"(%swift.bridge* %[[T1]], %swift.type* @"$sSiN")
2020
// CHECK-NEXT: %[[T5:.+]] = bitcast %TSo7NSArrayC* %[[T4]] to %TSo10CFArrayRefa*
21-
// CHECK-NEXT: call void asm sideeffect "", "r"(%TSo10CFArrayRefa* %[[T5]])
21+
// CHECK-NEXT: store %TSo10CFArrayRefa* %[[T5]]
2222
// CHECK-NEXT: call void @swift_bridgeObjectRelease(%swift.bridge* %[[T1]]) #{{[0-9]+}}
2323
// CHECK-NEXT: %[[T6:.+]] = bitcast %TSo10CFArrayRefa* %[[T5]] to i8*
2424
// CHECK-NEXT: call void @llvm.objc.release(i8* %[[T6]])

0 commit comments

Comments
 (0)