From b0585893ccb78508e900754c4974b0e95aafd4bf Mon Sep 17 00:00:00 2001 From: Vedant Kumar Date: Tue, 17 Apr 2018 22:03:08 +0000 Subject: [PATCH] [Mem2Reg] Create merged debug locations for inserted phis Track the debug locations of the incoming values to newly-created phis, and apply merged debug locations to the phis. A merged location will be on line 0, but will have the correct scope set. This improves crash reporting when an inlined instruction with a merged location triggers a machine exception. A debugger will be able to narrow down the crash to the correct inlined scope, instead of simply pointing to the outer scope of the caller. Taken together with a change allows generating merged line-0 locations for instructions which aren't calls, this results in a 0.5% increase in the uncompressed size of the .debug_line section of a stage2+Release build of clang (-O3 -g). rdar://33858697 Differential Revision: https://reviews.llvm.org/D45397 llvm-svn: 330227 --- .../Transforms/Utils/PromoteMemoryToRegister.cpp | 38 +++++-- .../Transforms/Mem2Reg/dbg-inline-scope-for-phi.ll | 110 +++++++++++++++++++++ 2 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 llvm/test/Transforms/Mem2Reg/dbg-inline-scope-for-phi.ll diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp index a0b68b6..562242e 100644 --- a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp +++ b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp @@ -167,13 +167,15 @@ struct AllocaInfo { /// Data package used by RenamePass(). struct RenamePassData { using ValVector = std::vector; + using LocationVector = std::vector; - RenamePassData(BasicBlock *B, BasicBlock *P, ValVector V) - : BB(B), Pred(P), Values(std::move(V)) {} + RenamePassData(BasicBlock *B, BasicBlock *P, ValVector V, LocationVector L) + : BB(B), Pred(P), Values(std::move(V)), Locations(std::move(L)) {} BasicBlock *BB; BasicBlock *Pred; ValVector Values; + LocationVector Locations; }; /// \brief This assigns and keeps a per-bb relative ordering of load/store @@ -302,6 +304,7 @@ private: SmallPtrSetImpl &LiveInBlocks); void RenamePass(BasicBlock *BB, BasicBlock *Pred, RenamePassData::ValVector &IncVals, + RenamePassData::LocationVector &IncLocs, std::vector &Worklist); bool QueuePhiNode(BasicBlock *BB, unsigned AllocaIdx, unsigned &Version); }; @@ -652,15 +655,20 @@ void PromoteMem2Reg::run() { for (unsigned i = 0, e = Allocas.size(); i != e; ++i) Values[i] = UndefValue::get(Allocas[i]->getAllocatedType()); + // When handling debug info, treat all incoming values as if they have unknown + // locations until proven otherwise. + RenamePassData::LocationVector Locations(Allocas.size()); + // Walks all basic blocks in the function performing the SSA rename algorithm // and inserting the phi nodes we marked as necessary std::vector RenamePassWorkList; - RenamePassWorkList.emplace_back(&F.front(), nullptr, std::move(Values)); + RenamePassWorkList.emplace_back(&F.front(), nullptr, std::move(Values), + std::move(Locations)); do { RenamePassData RPD = std::move(RenamePassWorkList.back()); RenamePassWorkList.pop_back(); // RenamePass may add new worklist entries. - RenamePass(RPD.BB, RPD.Pred, RPD.Values, RenamePassWorkList); + RenamePass(RPD.BB, RPD.Pred, RPD.Values, RPD.Locations, RenamePassWorkList); } while (!RenamePassWorkList.empty()); // The renamer uses the Visited set to avoid infinite loops. Clear it now. @@ -867,6 +875,16 @@ bool PromoteMem2Reg::QueuePhiNode(BasicBlock *BB, unsigned AllocaNo, return true; } +/// Update the debug location of a phi. \p ApplyMergedLoc indicates whether to +/// create a merged location incorporating \p DL, or to set \p DL directly. +static void updateForIncomingValueLocation(PHINode *PN, DebugLoc DL, + bool ApplyMergedLoc) { + if (ApplyMergedLoc) + PN->applyMergedLocation(PN->getDebugLoc(), DL); + else + PN->setDebugLoc(DL); +} + /// \brief Recursively traverse the CFG of the function, renaming loads and /// stores to the allocas which we are promoting. /// @@ -874,6 +892,7 @@ bool PromoteMem2Reg::QueuePhiNode(BasicBlock *BB, unsigned AllocaNo, /// predecessor block Pred. void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred, RenamePassData::ValVector &IncomingVals, + RenamePassData::LocationVector &IncomingLocs, std::vector &Worklist) { NextIteration: // If we are inserting any phi nodes into this BB, they will already be in the @@ -898,6 +917,10 @@ NextIteration: do { unsigned AllocaNo = PhiToAllocaMap[APN]; + // Update the location of the phi node. + updateForIncomingValueLocation(APN, IncomingLocs[AllocaNo], + APN->getNumIncomingValues() > 0); + // Add N incoming values to the PHI node. for (unsigned i = 0; i != NumEdges; ++i) APN->addIncoming(IncomingVals[AllocaNo], Pred); @@ -959,8 +982,11 @@ NextIteration: continue; // what value were we writing? - IncomingVals[ai->second] = SI->getOperand(0); + unsigned AllocaNo = ai->second; + IncomingVals[AllocaNo] = SI->getOperand(0); + // Record debuginfo for the store before removing it. + IncomingLocs[AllocaNo] = SI->getDebugLoc(); for (DbgInfoIntrinsic *DII : AllocaDbgDeclares[ai->second]) ConvertDebugDeclareToDebugValue(DII, SI, DIB); BB->getInstList().erase(SI); @@ -983,7 +1009,7 @@ NextIteration: for (; I != E; ++I) if (VisitedSuccs.insert(*I).second) - Worklist.emplace_back(*I, Pred, IncomingVals); + Worklist.emplace_back(*I, Pred, IncomingVals, IncomingLocs); goto NextIteration; } diff --git a/llvm/test/Transforms/Mem2Reg/dbg-inline-scope-for-phi.ll b/llvm/test/Transforms/Mem2Reg/dbg-inline-scope-for-phi.ll new file mode 100644 index 0000000..680f2dd --- /dev/null +++ b/llvm/test/Transforms/Mem2Reg/dbg-inline-scope-for-phi.ll @@ -0,0 +1,110 @@ +; RUN: opt -S < %s -mem2reg -verify | FileCheck %s + +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-apple-macosx10.13.0" + +; Original source (with some whitespace removed): +; +; extern int *getp(); +; extern int cond(); +; int get1() { return *getp(); } +; int get2(int *p) { return *p; } +; int bug(int *p) { +; if (cond()) return get1(); +; else return get2(p); +; } + +define i32 @get1() !dbg !8 { + %1 = call i32* (...) @getp(), !dbg !12 + %2 = load i32, i32* %1, align 4, !dbg !13 + ret i32 %2, !dbg !14 +} + +declare i32* @getp(...) + +define i32 @get2(i32*) !dbg !15 { + %2 = alloca i32*, align 8 + store i32* %0, i32** %2, align 8 + call void @llvm.dbg.declare(metadata i32** %2, metadata !19, metadata !DIExpression()), !dbg !20 + %3 = load i32*, i32** %2, align 8, !dbg !21 + %4 = load i32, i32* %3, align 4, !dbg !22 + ret i32 %4, !dbg !23 +} + +declare void @llvm.dbg.declare(metadata, metadata, metadata) + +; CHECK-LABEL: define i32 @bug +define i32 @bug(i32*) !dbg !24 { + %2 = alloca i32, align 4 + %3 = alloca i32*, align 8 + store i32* %0, i32** %3, align 8 + call void @llvm.dbg.declare(metadata i32** %3, metadata !25, metadata !DIExpression()), !dbg !26 + %4 = call i32 (...) @cond(), !dbg !27 + %5 = icmp ne i32 %4, 0, !dbg !27 + br i1 %5, label %6, label %8, !dbg !29 + +;