From b6cca0ec05cfc48f59628f6c4c076756b839f4a2 Mon Sep 17 00:00:00 2001 From: OCHyams Date: Thu, 27 Aug 2020 10:38:28 +0100 Subject: [PATCH] Revert "[DWARF] Add cuttoff guarding quadratic validThroughout behaviour" This reverts commit b9d977b0ca60c54f11615ca9d144c9f08b29fd85. This cutoff is no longer required. The commit 34ffa7fc501 (D86153) introduces a performance improvement which was tested against the motivating case for this patch. Discussed in differential revision: https://reviews.llvm.org/D86153 --- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 34 ++--------- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h | 6 +- .../DebugInfo/MIR/X86/singlelocation-cutoffs.mir | 65 ---------------------- 3 files changed, 8 insertions(+), 97 deletions(-) delete mode 100644 llvm/test/DebugInfo/MIR/X86/singlelocation-cutoffs.mir diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp index 3049e7c..64d57aa 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp @@ -148,11 +148,6 @@ static cl::opt "Abstract subprograms")), cl::init(DefaultLinkageNames)); -static cl::opt LocationAnalysisSizeLimit( - "singlevarlocation-input-bb-limit", - cl::desc("Maximum block size to analyze for single-location variables"), - cl::init(30000), cl::Hidden); - static const char *const DWARFGroupName = "dwarf"; static const char *const DWARFGroupDescription = "DWARF Emission"; static const char *const DbgTimerName = "writer"; @@ -1611,10 +1606,8 @@ static bool validThroughout(LexicalScopes &LScopes, // [1-3) [(reg0, fragment 0, 32), (reg1, fragment 32, 32)] // [3-4) [(reg1, fragment 32, 32), (123, fragment 64, 32)] // [4-) [(@g, fragment 0, 96)] -bool DwarfDebug::buildLocationList( - SmallVectorImpl &DebugLoc, - const DbgValueHistoryMap::Entries &Entries, - DenseSet &VeryLargeBlocks) { +bool DwarfDebug::buildLocationList(SmallVectorImpl &DebugLoc, + const DbgValueHistoryMap::Entries &Entries) { using OpenRange = std::pair; SmallVector OpenRanges; @@ -1710,14 +1703,8 @@ bool DwarfDebug::buildLocationList( DebugLoc.pop_back(); } - // If there's a single entry, safe for a single location, and not part of - // an over-sized basic block, then ask validThroughout whether this - // location can be represented as a single variable location. - if (DebugLoc.size() != 1 || !isSafeForSingleLocation) - return false; - if (VeryLargeBlocks.count(StartDebugMI->getParent())) - return false; - return validThroughout(LScopes, StartDebugMI, EndMI, getInstOrdering()); + return DebugLoc.size() == 1 && isSafeForSingleLocation && + validThroughout(LScopes, StartDebugMI, EndMI, getInstOrdering()); } DbgEntity *DwarfDebug::createConcreteEntity(DwarfCompileUnit &TheCU, @@ -1749,13 +1736,6 @@ void DwarfDebug::collectEntityInfo(DwarfCompileUnit &TheCU, // Grab the variable info that was squirreled away in the MMI side-table. collectVariableInfoFromMFTable(TheCU, Processed); - // Identify blocks that are unreasonably sized, so that we can later - // skip lexical scope analysis over them. - DenseSet VeryLargeBlocks; - for (const auto &MBB : *CurFn) - if (MBB.size() > LocationAnalysisSizeLimit) - VeryLargeBlocks.insert(&MBB); - for (const auto &I : DbgValues) { InlinedEntity IV = I.first; if (Processed.count(IV)) @@ -1792,8 +1772,7 @@ void DwarfDebug::collectEntityInfo(DwarfCompileUnit &TheCU, if (HistSize == 1 || SingleValueWithClobber) { const auto *End = SingleValueWithClobber ? HistoryMapEntries[1].getInstr() : nullptr; - if (VeryLargeBlocks.count(MInsn->getParent()) == 0 && - validThroughout(LScopes, MInsn, End, getInstOrdering())) { + if (validThroughout(LScopes, MInsn, End, getInstOrdering())) { RegVar->initializeDbgValue(MInsn); continue; } @@ -1808,8 +1787,7 @@ void DwarfDebug::collectEntityInfo(DwarfCompileUnit &TheCU, // Build the location list for this variable. SmallVector Entries; - bool isValidSingleLocation = - buildLocationList(Entries, HistoryMapEntries, VeryLargeBlocks); + bool isValidSingleLocation = buildLocationList(Entries, HistoryMapEntries); // Check whether buildLocationList managed to merge all locations to one // that is valid throughout the variable's scope. If so, produce single diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h index a97d368..ba0bb84 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h @@ -598,10 +598,8 @@ class DwarfDebug : public DebugHandlerBase { /// function that describe the same variable. If the resulting /// list has only one entry that is valid for entire variable's /// scope return true. - bool buildLocationList( - SmallVectorImpl &DebugLoc, - const DbgValueHistoryMap::Entries &Entries, - DenseSet &VeryLargeBlocks); + bool buildLocationList(SmallVectorImpl &DebugLoc, + const DbgValueHistoryMap::Entries &Entries); /// Collect variable information from the side table maintained by MF. void collectVariableInfoFromMFTable(DwarfCompileUnit &TheCU, diff --git a/llvm/test/DebugInfo/MIR/X86/singlelocation-cutoffs.mir b/llvm/test/DebugInfo/MIR/X86/singlelocation-cutoffs.mir deleted file mode 100644 index 6ad64d9..0000000 --- a/llvm/test/DebugInfo/MIR/X86/singlelocation-cutoffs.mir +++ /dev/null @@ -1,65 +0,0 @@ -# Test cutoffs for single-location variable analysis. -# Disable validThroughout if the input size exceeds the specified limit - -# RUN: llc %s -o - -start-after=livedebugvalues -mtriple=x86_64-unknown-unknown \ -# RUN: --singlevarlocation-input-bb-limit=0 -filetype=obj\ -# RUN: | llvm-dwarfdump -v -\ -# RUN: | FileCheck %s -check-prefix=LIMITED - -# RUN: llc %s -o - -start-after=livedebugvalues -mtriple=x86_64-unknown-unknown \ -# RUN: --singlevarlocation-input-bb-limit=20 -filetype=obj | llvm-dwarfdump -v -\ -# RUN: | FileCheck %s -check-prefix=UNLIMITED - -# LIMITED: DW_AT_location [DW_FORM_sec_offset] - -# UNLIMITED: DW_AT_location [DW_FORM_exprloc] - ---- | - target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" - - declare i32 @use(i32) - - define i32 @foo(i32 %x) !dbg !6 { - entry: - ret i32 1, !dbg !15 - } - - declare void @llvm.dbg.value(metadata, metadata, metadata) - - !llvm.dbg.cu = !{!0} - !llvm.debugify = !{!3, !4} - !llvm.module.flags = !{!5} - - !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2) - !1 = !DIFile(filename: "/tmp/t.ll", directory: "/") - !2 = !{} - !3 = !{i32 4} - !4 = !{i32 2} - !5 = !{i32 2, !"Debug Info Version", i32 3} - !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8) - !7 = !DISubroutineType(types: !2) - !8 = !{!9, !11} - !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10) - !10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned) - !11 = !DILocalVariable(name: "2", scope: !6, file: !1, line: 3, type: !10) - !12 = !DILocation(line: 1, column: 1, scope: !6) - !13 = !DILocation(line: 2, column: 1, scope: !6) - !14 = !DILocation(line: 3, column: 1, scope: !6) - !15 = !DILocation(line: 4, column: 1, scope: !6) - -... ---- -name: foo -liveins: - - { reg: '$edi', virtual-reg: '' } -stack: - - { id: 0, name: '', type: spill-slot, offset: -12, size: 4, alignment: 4, - stack-id: default, callee-saved-register: '', callee-saved-restored: true, - debug-info-variable: '', debug-info-expression: '', debug-info-location: '' } -body: | - bb.0.entry: - liveins: $edi - DBG_VALUE renamable $edi, $noreg, !11, !DIExpression(), debug-location !14 - RETQ debug-location !14 - -... -- 2.7.4