From 9d0dc96310eaedbf6a3ccc73e612685f1b950871 Mon Sep 17 00:00:00 2001 From: Andrew Ng Date: Fri, 28 Apr 2017 08:44:30 +0000 Subject: [PATCH] [DebugInfo][X86] Improve X86 Optimize LEAs handling of debug values. This is a follow up to the fix in r298360 to improve the handling of debug values when redundant LEAs are removed. The fix in r298360 effectively discarded the debug values. This patch now attempts to preserve the debug values by using the DWARF DW_OP_stack_value operation via prependDIExpr. Moved functions appendOffset and prependDIExpr from Local.cpp to DebugInfoMetadata.cpp and made them available as static member functions of DIExpression. Differential Revision: https://reviews.llvm.org/D31604 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@301630 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/DebugInfoMetadata.h | 11 +++ lib/IR/DebugInfoMetadata.cpp | 43 ++++++++++++ lib/Target/X86/X86OptimizeLEAs.cpp | 55 +++++++++++++-- lib/Transforms/Utils/Local.cpp | 53 ++------------ test/CodeGen/X86/lea-opt-with-debug.mir | 93 +++++++++++++------------ 5 files changed, 156 insertions(+), 99 deletions(-) diff --git a/include/llvm/IR/DebugInfoMetadata.h b/include/llvm/IR/DebugInfoMetadata.h index 67f51ccdeb3..6f2d1cb3d9d 100644 --- a/include/llvm/IR/DebugInfoMetadata.h +++ b/include/llvm/IR/DebugInfoMetadata.h @@ -56,6 +56,8 @@ namespace llvm { +class DIBuilder; + template class Optional; /// Holds a subclass of DINode. @@ -2275,6 +2277,15 @@ public: /// Return whether this is a piece of an aggregate variable. bool isFragment() const { return getFragmentInfo().hasValue(); } + + /// Append \p Ops with operations to apply the \p Offset. + static void appendOffset(SmallVectorImpl &Ops, int64_t Offset); + + /// Prepend \p DIExpr with a deref and offset operation and optionally turn it + /// into a stack value. + static DIExpression *prependDIExpr(DIBuilder &Builder, DIExpression *DIExpr, + bool Deref, int64_t Offset = 0, + bool StackValue = false); }; /// Global variables. diff --git a/lib/IR/DebugInfoMetadata.cpp b/lib/IR/DebugInfoMetadata.cpp index 3db9a3de92e..92f485323d5 100644 --- a/lib/IR/DebugInfoMetadata.cpp +++ b/lib/IR/DebugInfoMetadata.cpp @@ -15,6 +15,7 @@ #include "LLVMContextImpl.h" #include "MetadataImpl.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/IR/DIBuilder.h" #include "llvm/IR/Function.h" using namespace llvm; @@ -660,6 +661,48 @@ DIExpression::getFragmentInfo(expr_op_iterator Start, expr_op_iterator End) { return None; } +void DIExpression::appendOffset(SmallVectorImpl &Ops, + int64_t Offset) { + if (Offset > 0) { + Ops.push_back(dwarf::DW_OP_plus); + Ops.push_back(Offset); + } else if (Offset < 0) { + Ops.push_back(dwarf::DW_OP_minus); + Ops.push_back(-Offset); + } +} + +DIExpression * +DIExpression::prependDIExpr(DIBuilder &Builder, DIExpression *DIExpr, + bool Deref, int64_t Offset, + bool StackValue) { + if (!Deref && !Offset && !StackValue) + return DIExpr; + + SmallVector Ops; + appendOffset(Ops, Offset); + if (Deref) + Ops.push_back(dwarf::DW_OP_deref); + if (DIExpr) + for (auto Op : DIExpr->expr_ops()) { + // A DW_OP_stack_value comes at the end, but before a DW_OP_LLVM_fragment. + if (StackValue) { + if (Op.getOp() == dwarf::DW_OP_stack_value) + StackValue = false; + else if (Op.getOp() == dwarf::DW_OP_LLVM_fragment) { + Ops.push_back(dwarf::DW_OP_stack_value); + StackValue = false; + } + } + Ops.push_back(Op.getOp()); + for (unsigned I = 0; I < Op.getNumArgs(); ++I) + Ops.push_back(Op.getArg(I)); + } + if (StackValue) + Ops.push_back(dwarf::DW_OP_stack_value); + return Builder.createExpression(Ops); +} + bool DIExpression::isConstant() const { // Recognize DW_OP_constu C DW_OP_stack_value (DW_OP_LLVM_fragment Len Ofs)?. if (getNumElements() != 3 && getNumElements() != 6) diff --git a/lib/Target/X86/X86OptimizeLEAs.cpp b/lib/Target/X86/X86OptimizeLEAs.cpp index debb192732e..28c0757b2b6 100644 --- a/lib/Target/X86/X86OptimizeLEAs.cpp +++ b/lib/Target/X86/X86OptimizeLEAs.cpp @@ -27,6 +27,8 @@ #include "llvm/CodeGen/MachineOperand.h" #include "llvm/CodeGen/MachineRegisterInfo.h" #include "llvm/CodeGen/Passes.h" +#include "llvm/IR/DebugInfoMetadata.h" +#include "llvm/IR/DIBuilder.h" #include "llvm/IR/Function.h" #include "llvm/Support/Debug.h" #include "llvm/Support/raw_ostream.h" @@ -221,6 +223,8 @@ public: StringRef getPassName() const override { return "X86 LEA Optimize"; } + bool doInitialization(Module &M) override; + /// \brief Loop over all of the basic blocks, replacing address /// calculations in load and store instructions, if it's already /// been calculated by LEA. Also, remove redundant LEAs. @@ -262,6 +266,12 @@ private: /// \brief Removes redundant address calculations. bool removeRedundantAddrCalc(MemOpMap &LEAs); + /// Replace debug value MI with a new debug value instruction using register + /// VReg with an appropriate offset and DIExpression to incorporate the + /// address displacement AddrDispShift. Return new debug value instruction. + MachineInstr *replaceDebugValue(MachineInstr &MI, unsigned VReg, + int64_t AddrDispShift); + /// \brief Removes LEAs which calculate similar addresses. bool removeRedundantLEAs(MemOpMap &LEAs); @@ -270,6 +280,7 @@ private: MachineRegisterInfo *MRI; const X86InstrInfo *TII; const X86RegisterInfo *TRI; + Module *TheModule; static char ID; }; @@ -532,6 +543,26 @@ bool OptimizeLEAPass::removeRedundantAddrCalc(MemOpMap &LEAs) { return Changed; } +MachineInstr *OptimizeLEAPass::replaceDebugValue(MachineInstr &MI, + unsigned VReg, + int64_t AddrDispShift) { + DIExpression *Expr = const_cast(MI.getDebugExpression()); + + if (AddrDispShift != 0) { + DIBuilder DIB(*TheModule); + Expr = DIExpression::prependDIExpr(DIB, Expr, false, AddrDispShift, true); + } + + // Replace DBG_VALUE instruction with modified version. + MachineBasicBlock *MBB = MI.getParent(); + DebugLoc DL = MI.getDebugLoc(); + bool IsIndirect = MI.isIndirectDebugValue(); + int64_t Offset = IsIndirect ? MI.getOperand(1).getImm() : 0; + const MDNode *Var = MI.getDebugVariable(); + return BuildMI(*MBB, MBB->erase(&MI), DL, TII->get(TargetOpcode::DBG_VALUE), + IsIndirect, VReg, Offset, Var, Expr); +} + // Try to find similar LEAs in the list and replace one with another. bool OptimizeLEAPass::removeRedundantLEAs(MemOpMap &LEAs) { bool Changed = false; @@ -563,13 +594,21 @@ bool OptimizeLEAPass::removeRedundantLEAs(MemOpMap &LEAs) { // Loop over all uses of the Last LEA and update their operands. Note // that the correctness of this has already been checked in the // isReplaceable function. + unsigned FirstVReg = First.getOperand(0).getReg(); unsigned LastVReg = Last.getOperand(0).getReg(); - for (auto UI = MRI->use_nodbg_begin(LastVReg), - UE = MRI->use_nodbg_end(); + for (auto UI = MRI->use_begin(LastVReg), UE = MRI->use_end(); UI != UE;) { MachineOperand &MO = *UI++; MachineInstr &MI = *MO.getParent(); + if (MI.isDebugValue()) { + // Replace DBG_VALUE instruction with modified version using the + // register from the replacing LEA and the address displacement + // between the LEA instructions. + replaceDebugValue(MI, FirstVReg, AddrDispShift); + continue; + } + // Get the number of the first memory operand. const MCInstrDesc &Desc = MI.getDesc(); int MemOpNo = @@ -577,7 +616,7 @@ bool OptimizeLEAPass::removeRedundantLEAs(MemOpMap &LEAs) { X86II::getOperandBias(Desc); // Update address base. - MO.setReg(First.getOperand(0).getReg()); + MO.setReg(FirstVReg); // Update address disp. MachineOperand &Op = MI.getOperand(MemOpNo + X86::AddrDisp); @@ -587,11 +626,8 @@ bool OptimizeLEAPass::removeRedundantLEAs(MemOpMap &LEAs) { Op.setOffset(Op.getOffset() + AddrDispShift); } - // Mark debug values referring to Last LEA as undefined. - MRI->markUsesInDebugValueAsUndef(LastVReg); - // Since we can possibly extend register lifetime, clear kill flags. - MRI->clearKillFlags(First.getOperand(0).getReg()); + MRI->clearKillFlags(FirstVReg); ++NumRedundantLEAs; DEBUG(dbgs() << "OptimizeLEAs: Remove redundant LEA: "; Last.dump();); @@ -614,6 +650,11 @@ bool OptimizeLEAPass::removeRedundantLEAs(MemOpMap &LEAs) { return Changed; } +bool OptimizeLEAPass::doInitialization(Module &M) { + TheModule = &M; + return false; +} + bool OptimizeLEAPass::runOnMachineFunction(MachineFunction &MF) { bool Changed = false; diff --git a/lib/Transforms/Utils/Local.cpp b/lib/Transforms/Utils/Local.cpp index d3002c5fb75..1b643c53fe6 100644 --- a/lib/Transforms/Utils/Local.cpp +++ b/lib/Transforms/Utils/Local.cpp @@ -1259,50 +1259,8 @@ void llvm::findDbgValues(SmallVectorImpl &DbgValues, Value *V) { DbgValues.push_back(DVI); } -static void appendOffset(SmallVectorImpl &Ops, int64_t Offset) { - if (Offset > 0) { - Ops.push_back(dwarf::DW_OP_plus); - Ops.push_back(Offset); - } else if (Offset < 0) { - Ops.push_back(dwarf::DW_OP_minus); - Ops.push_back(-Offset); - } -} - enum { WithStackValue = true }; -/// Prepend \p DIExpr with a deref and offset operation and optionally turn it -/// into a stack value. -static DIExpression *prependDIExpr(DIBuilder &Builder, DIExpression *DIExpr, - bool Deref, int64_t Offset = 0, - bool StackValue = false) { - if (!Deref && !Offset && !StackValue) - return DIExpr; - - SmallVector Ops; - appendOffset(Ops, Offset); - if (Deref) - Ops.push_back(dwarf::DW_OP_deref); - if (DIExpr) - for (auto Op : DIExpr->expr_ops()) { - // A DW_OP_stack_value comes at the end, but before a DW_OP_LLVM_fragment. - if (StackValue) { - if (Op.getOp() == dwarf::DW_OP_stack_value) - StackValue = false; - else if (Op.getOp() == dwarf::DW_OP_LLVM_fragment) { - Ops.push_back(dwarf::DW_OP_stack_value); - StackValue = false; - } - } - Ops.push_back(Op.getOp()); - for (unsigned I = 0; I < Op.getNumArgs(); ++I) - Ops.push_back(Op.getArg(I)); - } - if (StackValue) - Ops.push_back(dwarf::DW_OP_stack_value); - return Builder.createExpression(Ops); -} - bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress, Instruction *InsertBefore, DIBuilder &Builder, bool Deref, int Offset) { @@ -1314,7 +1272,7 @@ bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress, auto *DIExpr = DDI->getExpression(); assert(DIVar && "Missing variable"); - DIExpr = prependDIExpr(Builder, DIExpr, Deref, Offset); + DIExpr = DIExpression::prependDIExpr(Builder, DIExpr, Deref, Offset); // Insert llvm.dbg.declare immediately after the original alloca, and remove // old llvm.dbg.declare. @@ -1348,7 +1306,7 @@ static void replaceOneDbgValueForAlloca(DbgValueInst *DVI, Value *NewAddress, if (Offset) { SmallVector Ops; Ops.push_back(dwarf::DW_OP_deref); - appendOffset(Ops, Offset); + DIExpression::appendOffset(Ops, Offset); Ops.append(DIExpr->elements_begin() + 1, DIExpr->elements_end()); DIExpr = Builder.createExpression(Ops); } @@ -1398,8 +1356,9 @@ void llvm::salvageDebugInfo(Instruction &I) { auto *DIExpr = DVI->getExpression(); DIBuilder DIB(M, /*AllowUnresolved*/ false); // GEP offsets are i32 and thus always fit into an int64_t. - DIExpr = prependDIExpr(DIB, DIExpr, NoDeref, Offset.getSExtValue(), - WithStackValue); + DIExpr = DIExpression::prependDIExpr(DIB, DIExpr, NoDeref, + Offset.getSExtValue(), + WithStackValue); DVI->setOperand(0, MDWrap(I.getOperand(0))); DVI->setOperand(3, MetadataAsValue::get(I.getContext(), DIExpr)); DEBUG(dbgs() << "SALVAGE: " << *DVI << '\n'); @@ -1411,7 +1370,7 @@ void llvm::salvageDebugInfo(Instruction &I) { // Rewrite the load into DW_OP_deref. auto *DIExpr = DVI->getExpression(); DIBuilder DIB(M, /*AllowUnresolved*/ false); - DIExpr = prependDIExpr(DIB, DIExpr, WithDeref); + DIExpr = DIExpression::prependDIExpr(DIB, DIExpr, WithDeref); DVI->setOperand(0, MDWrap(I.getOperand(0))); DVI->setOperand(3, MetadataAsValue::get(I.getContext(), DIExpr)); DEBUG(dbgs() << "SALVAGE: " << *DVI << '\n'); diff --git a/test/CodeGen/X86/lea-opt-with-debug.mir b/test/CodeGen/X86/lea-opt-with-debug.mir index ebf86ff718d..0a477706df1 100644 --- a/test/CodeGen/X86/lea-opt-with-debug.mir +++ b/test/CodeGen/X86/lea-opt-with-debug.mir @@ -1,7 +1,8 @@ -# RUN: llc -mtriple=x86_64-unknown-unknown -start-after peephole-opt -stop-before detect-dead-lanes -o - %s | FileCheck %s +# RUN: llc -mtriple=x86_64-unknown-unknown -start-after=peephole-opt -stop-before=detect-dead-lanes -o - %s | FileCheck %s -# Test that pass optimize LEA can remove a redundant LEA even when it is also -# used by a DBG_VALUE. +# Test that the optimize LEA pass can remove a redundant LEA even when it is +# also used by a DBG_VALUE. Check that the uses of the replaced LEA are updated +# correctly. --- | target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" @@ -13,22 +14,22 @@ @d = common local_unnamed_addr global i32 0, align 4 @b = common local_unnamed_addr global i32 0, align 4 - define i32 @fn1() local_unnamed_addr !dbg !8 { - %1 = load %struct.A*, %struct.A** @c, align 8, !dbg !13 - %2 = load i32, i32* @a, align 4, !dbg !13 - %3 = sext i32 %2 to i64, !dbg !13 - %4 = getelementptr inbounds %struct.A, %struct.A* %1, i64 %3, !dbg !13 - %5 = ptrtoint %struct.A* %4 to i64, !dbg !13 - %6 = trunc i64 %5 to i32, !dbg !13 - store i32 %6, i32* @d, align 4, !dbg !13 - %7 = getelementptr inbounds %struct.A, %struct.A* %1, i64 %3, i32 2, !dbg !14 - tail call void @llvm.dbg.value(metadata i32* %7, i64 0, metadata !11, metadata !15), !dbg !16 - br label %8, !dbg !17 + define i32 @fn1() local_unnamed_addr !dbg !9 { + %1 = load %struct.A*, %struct.A** @c, align 8, !dbg !14 + %2 = load i32, i32* @a, align 4, !dbg !14 + %3 = sext i32 %2 to i64, !dbg !14 + %4 = getelementptr inbounds %struct.A, %struct.A* %1, i64 %3, !dbg !14 + %5 = ptrtoint %struct.A* %4 to i64, !dbg !14 + %6 = trunc i64 %5 to i32, !dbg !14 + store i32 %6, i32* @d, align 4, !dbg !14 + %7 = getelementptr inbounds %struct.A, %struct.A* %1, i64 %3, i32 2, !dbg !15 + tail call void @llvm.dbg.value(metadata i32* %7, i64 0, metadata !12, metadata !16), !dbg !17 + br label %8, !dbg !18 ;