From dfaf133d27024395f9c8080252bc01126a6ee2be Mon Sep 17 00:00:00 2001 From: Davide Italiano Date: Wed, 28 Mar 2018 08:14:42 -0700 Subject: [PATCH] [LoadableByAddress] Fix debug info holes when expanding alloc_stack. --- include/swift/SIL/DebugUtils.h | 6 --- include/swift/SIL/SILBasicBlock.h | 5 ++ include/swift/SIL/SILBuilder.h | 9 ++++ include/swift/SIL/SILInstruction.h | 5 ++ lib/IRGen/LoadableByAddress.cpp | 17 ++++--- lib/SIL/SILBasicBlock.cpp | 8 +++ lib/SIL/SILInstruction.cpp | 14 ++++++ lib/SIL/SILVerifier.cpp | 4 +- .../LoadableByAddress-allockstack.swift | 49 +++++++++++++++++++ 9 files changed, 101 insertions(+), 16 deletions(-) create mode 100644 test/DebugInfo/LoadableByAddress-allockstack.swift diff --git a/include/swift/SIL/DebugUtils.h b/include/swift/SIL/DebugUtils.h index 621a113d8c28a..3df6dd5f3aa31 100644 --- a/include/swift/SIL/DebugUtils.h +++ b/include/swift/SIL/DebugUtils.h @@ -49,12 +49,6 @@ inline bool isDebugInst(SILInstruction *Inst) { return isa(Inst) || isa(Inst); } -/// Returns true if the instruction \p Inst is a maintenance instructions which -/// is relevant for debug information and does not get lowered to an instruction. -inline bool isMaintenanceInst(SILInstruction *Inst) { - return isDebugInst(Inst) || isa(Inst); -} - /// Deletes all of the debug instructions that use \p Inst. inline void deleteAllDebugUses(ValueBase *Inst) { for (auto UI = Inst->use_begin(), E = Inst->use_end(); UI != E;) { diff --git a/include/swift/SIL/SILBasicBlock.h b/include/swift/SIL/SILBasicBlock.h index 8fd8e18df28b7..84381b37b546a 100644 --- a/include/swift/SIL/SILBasicBlock.h +++ b/include/swift/SIL/SILBasicBlock.h @@ -344,6 +344,11 @@ public llvm::ilist_node, public SILAllocated { /// Used by llvm::LoopInfo. bool isLegalToHoistInto() const; + /// Returns the debug scope of the first non-meta instructions in the + /// basic block. SILBuilderWithScope uses this to correctly set up + /// the debug scope for newly created instructions. + const SILDebugScope *getScopeOfFirstNonMetaInstruction(); + //===--------------------------------------------------------------------===// // Debugging //===--------------------------------------------------------------------===// diff --git a/include/swift/SIL/SILBuilder.h b/include/swift/SIL/SILBuilder.h index dfe39e6e91315..7e5bd5a54afaf 100644 --- a/include/swift/SIL/SILBuilder.h +++ b/include/swift/SIL/SILBuilder.h @@ -2082,6 +2082,15 @@ class SILBuilderWithScope : public SILBuilder { : SILBuilder(BB) { inheritScopeFrom(InheritScopeFrom); } + + /// Creates a new SILBuilder with an insertion point at the + /// beginning of BB and the debug scope from the first + /// non-metainstruction in the BB. + explicit SILBuilderWithScope(SILBasicBlock *BB) : SILBuilder(BB->begin()) { + const SILDebugScope *DS = BB->getScopeOfFirstNonMetaInstruction(); + assert(DS && "Instruction without debug scope associated!"); + setCurrentDebugScope(DS); + } }; class SavedInsertionPointRAII { diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h index fffdcb9254c68..61330754cba7f 100644 --- a/include/swift/SIL/SILInstruction.h +++ b/include/swift/SIL/SILInstruction.h @@ -597,6 +597,11 @@ class SILInstruction /// you perform such optimizations like e.g. jump-threading. bool isTriviallyDuplicatable() const; + /// Returns true if the instruction is a meta instruction which is + /// relevant for debug information and does not get lowered to a real + /// instruction. + bool isMetaInstruction() const; + /// Verify that all operands of this instruction have compatible ownership /// with this instruction. void verifyOperandOwnership() const; diff --git a/lib/IRGen/LoadableByAddress.cpp b/lib/IRGen/LoadableByAddress.cpp index 4d63ace834041..0bf30bcc3f612 100644 --- a/lib/IRGen/LoadableByAddress.cpp +++ b/lib/IRGen/LoadableByAddress.cpp @@ -946,7 +946,7 @@ void LoadableStorageAllocation::replaceLoadWithCopyAddr( LoadInst *optimizableLoad) { SILValue value = optimizableLoad->getOperand(); - SILBuilderWithScope allocBuilder(pass.F->begin()->begin()); + SILBuilderWithScope allocBuilder(&*pass.F->begin()); AllocStackInst *allocInstr = allocBuilder.createAllocStack(value.getLoc(), value->getType()); @@ -1069,7 +1069,7 @@ void LoadableStorageAllocation::replaceLoadWithCopyAddrForModifiable( } SILValue value = unoptimizableLoad->getOperand(); - SILBuilderWithScope allocBuilder(pass.F->begin()->begin()); + SILBuilderWithScope allocBuilder(&*pass.F->begin()); AllocStackInst *allocInstr = allocBuilder.createAllocStack(value.getLoc(), value->getType()); @@ -1411,7 +1411,7 @@ void LoadableStorageAllocation::allocateForArg(SILValue value) { assert(!ApplySite::isa(value) && "Unexpected instruction"); - SILBuilderWithScope allocBuilder(pass.F->begin()->begin()); + SILBuilderWithScope allocBuilder(&*pass.F->begin()); AllocStackInst *allocInstr = allocBuilder.createAllocStack(value.getLoc(), value->getType()); @@ -1438,7 +1438,7 @@ void LoadableStorageAllocation::allocateForArg(SILValue value) { AllocStackInst * LoadableStorageAllocation::allocateForApply(SILInstruction *apply, SILType type) { - SILBuilderWithScope allocBuilder(pass.F->begin()->begin()); + SILBuilderWithScope allocBuilder(&*pass.F->begin()); auto *allocInstr = allocBuilder.createAllocStack(apply->getLoc(), type); pass.largeLoadableArgs.push_back(allocInstr); @@ -1524,7 +1524,7 @@ static void setInstrUsers(StructLoweringState &pass, AllocStackInst *allocInstr, static void allocateAndSetForInstrOperand(StructLoweringState &pass, SingleValueInstruction *instrOperand){ assert(instrOperand->getType().isObject()); - SILBuilderWithScope allocBuilder(pass.F->begin()->begin()); + SILBuilderWithScope allocBuilder(&*pass.F->begin()); AllocStackInst *allocInstr = allocBuilder.createAllocStack( instrOperand->getLoc(), instrOperand->getType()); @@ -1558,7 +1558,7 @@ static void allocateAndSetForArgumentOperand(StructLoweringState &pass, auto *arg = dyn_cast(value); assert(arg && "non-instr operand must be an argument"); - SILBuilderWithScope allocBuilder(pass.F->begin()->begin()); + SILBuilderWithScope allocBuilder(&*pass.F->begin()); AllocStackInst *allocInstr = allocBuilder.createAllocStack(applyInst->getLoc(), value->getType()); @@ -1677,7 +1677,8 @@ static SILValue createCopyOfEnum(StructLoweringState &pass, auto value = orig->getOperand(); auto type = value->getType(); if (type.isObject()) { - SILBuilderWithScope allocBuilder(pass.F->begin()->begin()); + SILBuilderWithScope allocBuilder(&*pass.F->begin()); + // support for non-address operands / enums auto *allocInstr = allocBuilder.createAllocStack(orig->getLoc(), type); SILBuilderWithScope storeBuilder(orig); @@ -1696,7 +1697,7 @@ static SILValue createCopyOfEnum(StructLoweringState &pass, } value = allocInstr; } - SILBuilderWithScope allocBuilder(pass.F->begin()->begin()); + SILBuilderWithScope allocBuilder(&*pass.F->begin()); auto *allocInstr = allocBuilder.createAllocStack(value.getLoc(), type); SILBuilderWithScope copyBuilder(orig); diff --git a/lib/SIL/SILBasicBlock.cpp b/lib/SIL/SILBasicBlock.cpp index 300230d0787f8..09c9a0bcc9dde 100644 --- a/lib/SIL/SILBasicBlock.cpp +++ b/lib/SIL/SILBasicBlock.cpp @@ -15,6 +15,7 @@ //===----------------------------------------------------------------------===// #include "llvm/ADT/STLExtras.h" +#include "swift/SIL/DebugUtils.h" #include "swift/SIL/SILBasicBlock.h" #include "swift/SIL/SILBuilder.h" #include "swift/SIL/SILArgument.h" @@ -380,3 +381,10 @@ bool SILBasicBlock::isTrampoline() const { bool SILBasicBlock::isLegalToHoistInto() const { return true; } + +const SILDebugScope *SILBasicBlock::getScopeOfFirstNonMetaInstruction() { + for (auto &Inst : *this) + if (Inst.isMetaInstruction()) + return Inst.getDebugScope(); + return begin()->getDebugScope(); +} diff --git a/lib/SIL/SILInstruction.cpp b/lib/SIL/SILInstruction.cpp index b9fca60de231b..fc514b8b467f7 100644 --- a/lib/SIL/SILInstruction.cpp +++ b/lib/SIL/SILInstruction.cpp @@ -1188,6 +1188,20 @@ bool SILInstruction::mayTrap() const { } } +bool SILInstruction::isMetaInstruction() const { + // Every instruction that implements getVarInfo() should be in this list. + switch (getKind()) { + case SILInstructionKind::AllocBoxInst: + case SILInstructionKind::AllocStackInst: + case SILInstructionKind::DebugValueInst: + case SILInstructionKind::DebugValueAddrInst: + return true; + default: + return false; + } + llvm_unreachable("Instruction not handled in isMetaInstruction()!"); +} + //===----------------------------------------------------------------------===// // Utilities //===----------------------------------------------------------------------===// diff --git a/lib/SIL/SILVerifier.cpp b/lib/SIL/SILVerifier.cpp index 287d6ab884d26..8d72c236dc550 100644 --- a/lib/SIL/SILVerifier.cpp +++ b/lib/SIL/SILVerifier.cpp @@ -4545,14 +4545,14 @@ class SILVerifier : public SILVerifierBase { const SILDebugScope *LastSeenScope = nullptr; for (SILInstruction &SI : *BB) { - if (isMaintenanceInst(&SI)) + if (SI.isMetaInstruction()) continue; LastSeenScope = SI.getDebugScope(); AlreadySeenScopes.insert(LastSeenScope); break; } for (SILInstruction &SI : *BB) { - if (isMaintenanceInst(&SI)) + if (SI.isMetaInstruction()) continue; // If we haven't seen this debug scope yet, update the diff --git a/test/DebugInfo/LoadableByAddress-allockstack.swift b/test/DebugInfo/LoadableByAddress-allockstack.swift new file mode 100644 index 0000000000000..a72c81fe48919 --- /dev/null +++ b/test/DebugInfo/LoadableByAddress-allockstack.swift @@ -0,0 +1,49 @@ +// Check we don't crash when verifying debug info. +// Ideally this should print the output after loadable by address runs +// but there's no way of doing this in SIL (for IRGen passes). +// RUN: %target-swift-frontend -emit-sil %s -Onone \ +// RUN: -sil-verify-all -Xllvm -verify-di-holes -emit-ir \ +// RUN: -Xllvm -sil-print-debuginfo -g -o - | %FileCheck %s + +struct m { + let major: Int + let minor: Int + let n: Int + let o: [String] + let p: [String] + init(major: Int, minor: Int, n: Int, o: [String], p: [String]) { + self.major = major + self.minor = minor + self.n = n + self.o = o + self.p = p + } +} + +enum a { + case any + case b(m) +} + +struct c { + enum f { + case g(a) + } +} + +struct h{ + typealias j = i + typealias d = j + typealias f = c.f + subscript(identifier: d) -> f { + return .g(.any) + } + func k(l: f, identifier: d) -> h { + switch (l, self[identifier]) { + default: + return self + } + } +} + +// CHECK: define linkonce_odr hidden %swift.opaque* @"$S4main1mVwCP"