[llvm-commits] [llvm] r128327 - in /llvm/trunk: include/llvm/CodeGen/MachineBasicBlock.h lib/CodeGen/AsmPrinter/DwarfDebug.cpp lib/CodeGen/AsmPrinter/DwarfDebug.h test/CodeGen/X86/dbg-merge-loc-entry.ll
David Blaikie
dblaikie at gmail.com
Mon Dec 2 23:16:37 PST 2013
On Mon, Dec 2, 2013 at 8:00 AM, Alexey Samsonov <samsonov at google.com> wrote:
> Hi David,
>
> Let me outline some ideas I got after reading this code as a
> sanity-check-request - I have too little knowledge of DBG_VALUE machine
> instructions, working with machine functions, etc.
>
> On Fri, Nov 1, 2013 at 1:46 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> To document some of my own state on this issue (I'm happy to take this
>> into another thread or bug to discuss if people think there's a better
>> context for such discussion)...
>>
>> In DwarfDebug::beginFunction there is a bunch of code (that could probably
>> be refactored into some named functions to make it more documented) but
>> here's the basic idea:
>>
>> For all MBB
>> For all MI
>> if it's a debug value
>> record the location in the 'history' for this variable
>> mark the register as in-use by a certain variable (what happens if
>> we end up with two variables in the same register? I don't think this code
>> can handle that)
>
>
> I agree - there are real-life cases when several variables refer to the same
> register - e.g. in ASan mode addresses of all local variables are defined as
> offsets from $rsp via DBG_VALUE instruction.
> So, instead of [Register]->[Debug variable] mapping we need to have
> [Register] -> [Vector of debug variables] here.
Fair point. Though are we ever going to indirect through transient
registers? (eg: "variable is in deref(reg+offset) where 'reg' isn't
just the frame register live across the whole function)
If the only indirection is for stack cases, then we don't need to
worry about clobbers I think - well, not in the traditional sense the
way this algorithm is written. We might need to worry about stack
reuse... which could get interesting. I'm not sure how that (lifetime
markers) is modeled.
> The code in this branch also inserts a range-terminator instruction between
> a pair DBG_VALUE instrs for the same variable if instructions in this pair
> are from different basic blocks (I assume, this is done for the same
> purpose as inserting range-terminators in the second loop, and it can hurt
> us as well).
Right - they're both parts of the same general algorithm to terminate
at the end of BBs. There's no actual DBG_VALUE to terminate here, it's
just implicit in this algorithm that the live ranges terminate at the
BB boundary.
>> else
>> if this instruction clobbers any register
>> if this instruction is in the same MBB as the current defining
>> debug value for the variable
>> record this instruction in the history (it will act as a debug
>> loc range terminator) of the variable
>>
>> For all the variables
>> If the end of the history is debug value (not a non-debug instruction
>> which would terminate the range)
>> If that debug value is in a different basic block than the final basic
>> block in the function
>> add the last instruction in the debug values basic block to the
>> history to terminate the range
>>
>
> We terminate ranges too early - some registers may not in fact be clobbered
> between two different basic blocks. However, looks like we *have* all the
> information we need - DBG_VALUE instructions and
> register-clobbering instructions - in DwarfDebug::beginFunction(), and
> should just properly compute location ranges. It is nontrivial, though, and
> I think the main purpose range-terminator instructions were added is
> to deal with confusion when MBB has more than one predecessor. We need to
> somehow implement "phi-nodes" for variable locations...
> Suppose we have:
>
> MBB1:
> dbg_loc("x") = %rax
> jmp MBB3
> MBB2:
> dbg_loc("x") = %rdx
> jmp MBB3
> MBB3:
> // where is "x" now?
>
> currently we add dbg_loc("x") = undef at the end of MBB1 and MBB2 for
> safety, but in general we should do a proper analysis.
My thinking is that we shouldn't need to perform an analysis here - we
should rely on the register allocator to tell us what it already knows
about liveness. Recomputing this from an MBB analysis seems like we'd
be redoing work that's already been performed.
Thing is, things like the frame pointer are sort of "beyond" register
allocation - their blessing and immutability is an underlying
assumption of the whole system. So I'm wondering if the right thing to
do here is to never model frame index and frame pointer reg+offsets as
dbg_values and instead model them in the same way we model allocas
(indeed we could generalize the alloca handling, hopefully).
This won't solve our general debug info location problems at "above
-O0" where non-frame registers may remain live over basic block
boundaries in novel ways. And that will probably need to be solved one
day - but I'm starting to think that the solution to that still won't
generalize over/solve the stack slot problem we have today. Not that I
know much about this stuff, but I /think/ stack slots are treated so
separately that it wouldn't fall out of the
register-allocator-aware-location stuff we'll need for optimized debug
locations in non-frame registers.
> Now this looks like a problem, which could be solved by graph traversal :)
> Basically, we may take first instruction and all the DBG_VALUE instructions
> as starting points, and scan all the instructions in the order of execution
> (following jumps), inserting range-terminators if we see a register
> clobbering instruction or if we enter a new MBB and incoming variable
> locations are different.
>
> Does it all make sense?
>
>
>> Hopefully that makes some sense. The bolded text is one of the problematic
>> places - but I don't believe it's correct to simply not do that check (some
>> things do actually get clobbered across basic blocks). It might be as simple
>> as also checking (both at the bolded step, and in the "for all variables"
>> loop) if the register in question is something that won't get clobbered
>> across basic blocks.
>>
>> Lang - this still seems rather special-cased. I assume we do some inter-BB
>> register allocation above -O0, so how should we be doing this to get
>> accurate live ranges for our debug variables? Just assuming all registers
>> (except the base pointer?) get clobbered at the end of a basic block seems a
>> bit 'rough' at best, but perhaps I'm wrong.
>>
>> - David
>>
>> On Tue, Oct 29, 2013 at 3:33 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>> +Alexey who's encountering this in ASan
>>>
>>>
>>> On Fri, Oct 25, 2013 at 1:28 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>>
>>>> Bump because... argh. (I've also reformatted the samples to remove all
>>>> the extra newlines, hopefully).
>>>>
>>>> Lang - you want to chat about this some time? Eric & I really aren't
>>>> sure who else to chat to about this.
>>>>
>>>>> Eric - this patch seems to be related to the issue I was discussing
>>>>> at/with you yesterday. Specifically the "or when the basic block ends" part
>>>>> is problematic for cases I'm looking at:
>>>>
>>>>
>>>>>
>>>>> struct foo {
>>>>> foo() : i(3) {}
>>>>> foo(const foo& f) : i(f.i) { }
>>>>> int i;
>>>>> };
>>>>> int func(foo f) {
>>>>> if (!f.i)
>>>>> return f.i + 1;
>>>>> return f.i + 2;
>>>>> }
>>>>> int main() {
>>>>> foo l;
>>>>> func(l);
>>>>> }
>>>>>
>>>>>
>>>>> I get the following assembly for 'func':
>>>>>
>>>>> .globl _Z4func3foo
>>>>> .align 16, 0x90
>>>>> .type _Z4func3foo, at function
>>>>> _Z4func3foo: # @_Z4func3foo
>>>>> .cfi_startproc
>>>>> .Lfunc_begin0:
>>>>> .loc 1 7 0 # test.cpp:7:0
>>>>> # BB#0: # %entry
>>>>> pushq %rbp
>>>>> .Ltmp2:
>>>>> .cfi_def_cfa_offset 16
>>>>> .Ltmp3:
>>>>> .cfi_offset %rbp, -16
>>>>> movq %rsp, %rbp
>>>>> .Ltmp4:
>>>>> .cfi_def_cfa_register %rbp
>>>>> #DEBUG_VALUE: func:f <- RDI
>>>>> .loc 1 8 0 prologue_end # test.cpp:8:0
>>>>> .Ltmp5:
>>>>> cmpl $0, (%rdi)
>>>>> movq %rdi, -16(%rbp) # 8-byte Spill
>>>>> .Ltmp6: *no
>>>>> #DEBUG_VALUE: func:f <- [RBP+-16]
>>>>> jne .LBB0_2
>>>>> .Ltmp7:
>>>>> # BB#1: # %if.then
>>>>> .loc 1 9 0 # test.cpp:9:0
>>>>> movq -16(%rbp), %rax # 8-byte Reload
>>>>> movl (%rax), %ecx
>>>>> addl $1, %ecx
>>>>> movl %ecx, -4(%rbp)
>>>>> jmp .LBB0_3
>>>>> .Ltmp8:
>>>>> .LBB0_2: # %if.end
>>>>> .loc 1 10 0 # test.cpp:10:0
>>>>> movq -16(%rbp), %rax # 8-byte Reload
>>>>> movl (%rax), %ecx
>>>>> addl $2, %ecx
>>>>> movl %ecx, -4(%rbp)
>>>>> .LBB0_3: # %return
>>>>> .loc 1 11 0 # test.cpp:11:0
>>>>> movl -4(%rbp), %eax
>>>>> popq %rbp
>>>>> ret
>>>>
>>>>
>>>>>
>>>>> And the debug_loc for 'func:f' is:
>>>>>
>>>>> .Ldebug_loc0:
>>>>> .quad .Lfunc_begin0
>>>>> .quad .Ltmp6
>>>>> .Lset0 = .Ltmp53-.Ltmp52 # Loc expr size
>>>>> .short .Lset0
>>>>> .Ltmp52:
>>>>> .byte 117 # DW_OP_breg5
>>>>> .byte 0
>>>>> .Ltmp53:
>>>>> .quad .Ltmp6
>>>>> .quad .Ltmp7
>>>>> .Lset1 = .Ltmp55-.Ltmp54 # Loc expr size
>>>>> .short .Lset1
>>>>> .Ltmp54:
>>>>> .byte 118 # DW_OP_breg6
>>>>> .byte 112
>>>>> .byte 6
>>>>> .Ltmp55:
>>>>>
>>>>>
>>>>> The important point being that the second range for the variable (for
>>>>> -16(%rbp)) ends at the end of the first basic block. Thus for the range
>>>>> tmp7-func_end we have no location information for this variable.
>>>>>
>>>>> This bug appears to manifest on any non-trivial-pass-by-value parameter
>>>>> and any trivial pass-by-value parameter than ends up lowered to LLVM "byval"
>>>>> (if it's split into multiple reg parameters then we must reconstitute it
>>>>> inside the function and then we track the debug info for that reconstituted
>>>>> value - probably an alloca so everything is good at -O0 at least).
>>>>>
>>>>> Should we be special casing indirect dbg_values and letting them past
>>>>> the edge of basic blocks?
>>>>> Or only ones relative to the base pointer?
>>>>> Or do we need to have dbg_value MI intrinsics inserted at the beginning
>>>>> of every subsequent basic block to properly communicate where variables are?
>>>>> Or something else entirely.
>>>>>
>>>>>> The code is now ready to deal with variables that are sometimes in a
>>>>>> register
>>>>>> and sometimes on the stack. We just need to teach emitDebugLoc to say
>>>>>> 'stack
>>>>>> slot'.
>>>>>
>>>>>
>>>>> Not sure if this /\ point is relevant, irrelevant, or something that's
>>>>> been addressed since this commit. My best guess is that this comment is just
>>>>> about how we describe stack slots in a location list - in that we could
>>>>> describe them more compactly than reg+offset (by using fbreg instead of
>>>>> breg) and we aren't. That seems like an easy special case of "is the
>>>>> register the frame register, then use fbreg".
>>>>>
>>>>>>
>>>>>>
>>>>>> Modified:
>>>>>> llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h
>>>>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>>>>> llvm/trunk/test/CodeGen/X86/dbg-merge-loc-entry.ll
>>>>>>
>>>>>> Modified: llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h?rev=128327&r1=128326&r2=128327&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h (original)
>>>>>> +++ llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h Fri Mar 25
>>>>>> 21:19:36 2011
>>>>>> @@ -309,6 +309,10 @@
>>>>>> /// instruction in the basic block, or end()
>>>>>> iterator getLastNonDebugInstr();
>>>>>>
>>>>>> + const_iterator getLastNonDebugInstr() const {
>>>>>> + return
>>>>>> const_cast<MachineBasicBlock*>(this)->getLastNonDebugInstr();
>>>>>> + }
>>>>>> +
>>>>>> /// SplitCriticalEdge - Split the critical edge from this block to
>>>>>> the
>>>>>> /// given successor block, and return the newly created block, or
>>>>>> null
>>>>>> /// if splitting is not possible.
>>>>>>
>>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=128327&r1=128326&r2=128327&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Fri Mar 25
>>>>>> 21:19:36 2011
>>>>>> @@ -2408,38 +2408,21 @@
>>>>>> /// collection info from MMI table.
>>>>>> collectVariableInfoFromMMITable(MF, Processed);
>>>>>>
>>>>>> - SmallVector<const MachineInstr *, 8> DbgValues;
>>>>>> - // Collect variable information from DBG_VALUE machine
>>>>>> instructions;
>>>>>> - for (MachineFunction::const_iterator I = Asm->MF->begin(), E =
>>>>>> Asm->MF->end();
>>>>>> - I != E; ++I)
>>>>>> - for (MachineBasicBlock::const_iterator II = I->begin(), IE =
>>>>>> I->end();
>>>>>> - II != IE; ++II) {
>>>>>> - const MachineInstr *MInsn = II;
>>>>>> - if (!MInsn->isDebugValue())
>>>>>> - continue;
>>>>>> - DbgValues.push_back(MInsn);
>>>>>> - }
>>>>>> -
>>>>>> - // This is a collection of DBG_VALUE instructions describing same
>>>>>> variable.
>>>>>> - SmallVector<const MachineInstr *, 4> MultipleValues;
>>>>>> - for(SmallVector<const MachineInstr *, 8>::iterator I =
>>>>>> DbgValues.begin(),
>>>>>> - E = DbgValues.end(); I != E; ++I) {
>>>>>> - const MachineInstr *MInsn = *I;
>>>>>> - MultipleValues.clear();
>>>>>> - if (isDbgValueInDefinedReg(MInsn))
>>>>>> - MultipleValues.push_back(MInsn);
>>>>>> - DIVariable DV(MInsn->getOperand(MInsn->getNumOperands() -
>>>>>> 1).getMetadata());
>>>>>> - if (Processed.count(DV) != 0)
>>>>>> + for (SmallVectorImpl<const MDNode*>::const_iterator
>>>>>> + UVI = UserVariables.begin(), UVE = UserVariables.end(); UVI
>>>>>> != UVE;
>>>>>> + ++UVI) {
>>>>>> + const MDNode *Var = *UVI;
>>>>>> + if (Processed.count(Var))
>>>>>> continue;
>>>>>>
>>>>>> - for (SmallVector<const MachineInstr *, 8>::iterator MI = I+1,
>>>>>> - ME = DbgValues.end(); MI != ME; ++MI) {
>>>>>> - const MDNode *Var =
>>>>>> - (*MI)->getOperand((*MI)->getNumOperands()-1).getMetadata();
>>>>>> - if (Var == DV)
>>>>>> - MultipleValues.push_back(*MI);
>>>>>> - }
>>>>>> + // History contains relevant DBG_VALUE instructions for Var and
>>>>>> instructions
>>>>>> + // clobbering it.
>>>>>> + SmallVectorImpl<const MachineInstr*> &History = DbgValues[Var];
>>>>>> + if (History.empty())
>>>>>> + continue;
>>>>>> + const MachineInstr *MInsn = History.front();
>>>>>>
>>>>>> + DIVariable DV(Var);
>>>>>> DbgScope *Scope = NULL;
>>>>>> if (DV.getTag() == dwarf::DW_TAG_arg_variable &&
>>>>>> DISubprogram(DV.getContext()).describes(MF->getFunction()))
>>>>>> @@ -2451,6 +2434,7 @@
>>>>>> continue;
>>>>>>
>>>>>> Processed.insert(DV);
>>>>>> + assert(MInsn->isDebugValue() && "History must begin with debug
>>>>>> value");
>>>>>> DbgVariable *RegVar = new DbgVariable(DV);
>>>>>> if (!addCurrentFnArgument(MF, RegVar, Scope))
>>>>>> Scope->addVariable(RegVar);
>>>>>> @@ -2458,21 +2442,21 @@
>>>>>> DbgVariableToDbgInstMap[AbsVar] = MInsn;
>>>>>> VarToAbstractVarMap[RegVar] = AbsVar;
>>>>>> }
>>>>>> - if (MultipleValues.size() <= 1 && !RegClobberInsn.count(MInsn)) {
>>>>>> +
>>>>>> + // Simple ranges that are fully coalesced.
>>>>>> + if (History.size() <= 1 || (History.size() == 2 &&
>>>>>> +
>>>>>> MInsn->isIdenticalTo(History.back()))) {
>>>>>> DbgVariableToDbgInstMap[RegVar] = MInsn;
>>>>>> continue;
>>>>>> }
>>>>>>
>>>>>> // handle multiple DBG_VALUE instructions describing one
>>>>>> variable.
>>>>>> - if (DotDebugLocEntries.empty())
>>>>>> - RegVar->setDotDebugLocOffset(0);
>>>>>> - else
>>>>>> - RegVar->setDotDebugLocOffset(DotDebugLocEntries.size());
>>>>>> + RegVar->setDotDebugLocOffset(DotDebugLocEntries.size());
>>>>>>
>>>>>> - for (SmallVector<const MachineInstr *, 4>::iterator
>>>>>> - MVI = MultipleValues.begin(), MVE = MultipleValues.end();
>>>>>> - MVI != MVE; ++MVI) {
>>>>>> - const MachineInstr *Begin = *MVI;
>>>>>> + for (SmallVectorImpl<const MachineInstr*>::const_iterator
>>>>>> + HI = History.begin(), HE = History.end(); HI != HE; ++HI)
>>>>>> {
>>>>>> + const MachineInstr *Begin = *HI;
>>>>>> + assert(Begin->isDebugValue() && "Invalid History entry");
>>>>>> MachineLocation MLoc;
>>>>>> if (Begin->getNumOperands() == 3) {
>>>>>> if (Begin->getOperand(0).isReg() &&
>>>>>> Begin->getOperand(1).isImm())
>>>>>> @@ -2480,6 +2464,7 @@
>>>>>> } else
>>>>>> MLoc = Asm->getDebugValueLocation(Begin);
>>>>>>
>>>>>> + // FIXME: emitDebugLoc only understands registers.
>>>>>> if (!MLoc.getReg())
>>>>>> continue;
>>>>>>
>>>>>> @@ -2487,17 +2472,23 @@
>>>>>> const MCSymbol *FLabel = getLabelBeforeInsn(Begin);
>>>>>> const MCSymbol *SLabel = 0;
>>>>>>
>>>>>> - if (const MachineInstr *ClobberMI =
>>>>>> RegClobberInsn.lookup(Begin))
>>>>>> - // The register range starting at Begin may be clobbered.
>>>>>> - SLabel = getLabelAfterInsn(ClobberMI);
>>>>>> - else if (MVI + 1 == MVE)
>>>>>> - // If Begin is the last instruction then its value is valid
>>>>>> + if (HI + 1 == HE)
>>>>>> + // If Begin is the last instruction in History then its value
>>>>>> is valid
>>>>>> // until the end of the funtion.
>>>>>> SLabel = FunctionEndSym;
>>>>>> - else
>>>>>> - // The value is valid until the next DBG_VALUE.
>>>>>> - SLabel = getLabelBeforeInsn(MVI[1]);
>>>>>> + else {
>>>>>> + const MachineInstr *End = HI[1];
>>>>>> + if (End->isDebugValue())
>>>>>> + SLabel = getLabelBeforeInsn(End);
>>>>>> + else {
>>>>>> + // End is a normal instruction clobbering the range.
>>>>>> + SLabel = getLabelAfterInsn(End);
>>>>>> + assert(SLabel && "Forgot label after clobber instruction");
>>>>>> + ++HI;
>>>>>> + }
>>>>>> + }
>>>>>>
>>>>>> + // The value is valid until the next DBG_VALUE or clobber.
>>>>>> DotDebugLocEntries.push_back(DotDebugLocEntry(FLabel, SLabel,
>>>>>> MLoc));
>>>>>> }
>>>>>> DotDebugLocEntries.push_back(DotDebugLocEntry());
>>>>>> @@ -2519,21 +2510,14 @@
>>>>>>
>>>>>> /// getLabelBeforeInsn - Return Label preceding the instruction.
>>>>>> const MCSymbol *DwarfDebug::getLabelBeforeInsn(const MachineInstr
>>>>>> *MI) {
>>>>>> - DenseMap<const MachineInstr *, MCSymbol *>::iterator I =
>>>>>> - LabelsBeforeInsn.find(MI);
>>>>>> - if (I == LabelsBeforeInsn.end())
>>>>>> - // FunctionBeginSym always preceeds all the instruction in
>>>>>> current function.
>>>>>> - return FunctionBeginSym;
>>>>>> - return I->second;
>>>>>> + MCSymbol *Label = LabelsBeforeInsn.lookup(MI);
>>>>>> + assert(Label && "Didn't insert label before instruction");
>>>>>> + return Label;
>>>>>> }
>>>>>>
>>>>>> /// getLabelAfterInsn - Return Label immediately following the
>>>>>> instruction.
>>>>>> const MCSymbol *DwarfDebug::getLabelAfterInsn(const MachineInstr *MI)
>>>>>> {
>>>>>> - DenseMap<const MachineInstr *, MCSymbol *>::iterator I =
>>>>>> - LabelsAfterInsn.find(MI);
>>>>>> - if (I == LabelsAfterInsn.end())
>>>>>> - return NULL;
>>>>>> - return I->second;
>>>>>> + return LabelsAfterInsn.lookup(MI);
>>>>>> }
>>>>>>
>>>>>> /// beginInstruction - Process beginning of an instruction.
>>>>>> @@ -2552,14 +2536,22 @@
>>>>>> }
>>>>>>
>>>>>> // Insert labels where requested.
>>>>>> - if (!InsnNeedsLabel.count(MI))
>>>>>> + DenseMap<const MachineInstr*, MCSymbol*>::iterator I =
>>>>>> + LabelsBeforeInsn.find(MI);
>>>>>> +
>>>>>> + // No label needed.
>>>>>> + if (I == LabelsBeforeInsn.end())
>>>>>> + return;
>>>>>> +
>>>>>> + // Label already assigned.
>>>>>> + if (I->second)
>>>>>> return;
>>>>>>
>>>>>> if (!PrevLabel) {
>>>>>> PrevLabel = MMI->getContext().CreateTempSymbol();
>>>>>> Asm->OutStreamer.EmitLabel(PrevLabel);
>>>>>> }
>>>>>> - LabelsBeforeInsn[MI] = PrevLabel;
>>>>>> + I->second = PrevLabel;
>>>>>> }
>>>>>>
>>>>>> /// endInstruction - Process end of an instruction.
>>>>>> @@ -2569,7 +2561,15 @@
>>>>>> if (!MI->isDebugValue())
>>>>>> PrevLabel = 0;
>>>>>>
>>>>>> - if (!InsnsNeedsLabelAfter.count(MI))
>>>>>> + DenseMap<const MachineInstr*, MCSymbol*>::iterator I =
>>>>>> + LabelsAfterInsn.find(MI);
>>>>>> +
>>>>>> + // No label needed.
>>>>>> + if (I == LabelsAfterInsn.end())
>>>>>> + return;
>>>>>> +
>>>>>> + // Label already assigned.
>>>>>> + if (I->second)
>>>>>> return;
>>>>>>
>>>>>> // We need a label after this instruction.
>>>>>> @@ -2577,7 +2577,7 @@
>>>>>> PrevLabel = MMI->getContext().CreateTempSymbol();
>>>>>> Asm->OutStreamer.EmitLabel(PrevLabel);
>>>>>> }
>>>>>> - LabelsAfterInsn[MI] = PrevLabel;
>>>>>> + I->second = PrevLabel;
>>>>>> }
>>>>>>
>>>>>> /// getOrCreateDbgScope - Create DbgScope for the scope.
>>>>>> @@ -2837,8 +2837,8 @@
>>>>>> RE = Ranges.end(); RI != RE; ++RI) {
>>>>>> assert(RI->first && "DbgRange does not have first
>>>>>> instruction!");
>>>>>> assert(RI->second && "DbgRange does not have second
>>>>>> instruction!");
>>>>>> - InsnNeedsLabel.insert(RI->first);
>>>>>> - InsnsNeedsLabelAfter.insert(RI->second);
>>>>>> + requestLabelBeforeInsn(RI->first);
>>>>>> + requestLabelAfterInsn(RI->second);
>>>>>> }
>>>>>> }
>>>>>> }
>>>>>> @@ -2916,46 +2916,78 @@
>>>>>>
>>>>>> recordSourceLine(Line, Col, TheScope);
>>>>>>
>>>>>> + assert(UserVariables.empty() && DbgValues.empty() && "Maps weren't
>>>>>> cleaned");
>>>>>> +
>>>>>> /// ProcessedArgs - Collection of arguments already processed.
>>>>>> SmallPtrSet<const MDNode *, 8> ProcessedArgs;
>>>>>>
>>>>>> - /// LastDbgValue - Refer back to the last DBG_VALUE instruction to
>>>>>> mention MD.
>>>>>> - DenseMap<const MDNode*, const MachineInstr*> LastDbgValue;
>>>>>> -
>>>>>> const TargetRegisterInfo *TRI = Asm->TM.getRegisterInfo();
>>>>>>
>>>>>> /// LiveUserVar - Map physreg numbers to the MDNode they contain.
>>>>>> std::vector<const MDNode*> LiveUserVar(TRI->getNumRegs());
>>>>>>
>>>>>> for (MachineFunction::const_iterator I = MF->begin(), E =
>>>>>> MF->end();
>>>>>> - I != E; ++I)
>>>>>> + I != E; ++I) {
>>>>>> + bool AtBlockEntry = true;
>>>>>> for (MachineBasicBlock::const_iterator II = I->begin(), IE =
>>>>>> I->end();
>>>>>> II != IE; ++II) {
>>>>>> const MachineInstr *MI = II;
>>>>>> - DebugLoc DL = MI->getDebugLoc();
>>>>>> +
>>>>>> if (MI->isDebugValue()) {
>>>>>> assert (MI->getNumOperands() > 1 && "Invalid machine
>>>>>> instruction!");
>>>>>>
>>>>>> - // Keep track of variables in registers.
>>>>>> + // Keep track of user variables.
>>>>>> const MDNode *Var =
>>>>>> MI->getOperand(MI->getNumOperands() - 1).getMetadata();
>>>>>> - LastDbgValue[Var] = MI;
>>>>>> +
>>>>>> + // Variable is in a register, we need to check for clobbers.
>>>>>> if (isDbgValueInDefinedReg(MI))
>>>>>> LiveUserVar[MI->getOperand(0).getReg()] = Var;
>>>>>>
>>>>>> - DIVariable DV(Var);
>>>>>> - if (!DV.Verify()) continue;
>>>>>> - // If DBG_VALUE is for a local variable then it needs a
>>>>>> label.
>>>>>> - if (DV.getTag() != dwarf::DW_TAG_arg_variable)
>>>>>> - InsnNeedsLabel.insert(MI);
>>>>>> - // DBG_VALUE for inlined functions argument needs a label.
>>>>>> - else if (!DISubprogram(getDISubprogram(DV.getContext())).
>>>>>> - describes(MF->getFunction()))
>>>>>> - InsnNeedsLabel.insert(MI);
>>>>>> - // DBG_VALUE indicating argument location change needs a
>>>>>> label.
>>>>>> - else if (!ProcessedArgs.insert(DV))
>>>>>> - InsnNeedsLabel.insert(MI);
>>>>>> + // Check the history of this variable.
>>>>>> + SmallVectorImpl<const MachineInstr*> &History =
>>>>>> DbgValues[Var];
>>>>>> + if (History.empty()) {
>>>>>> + UserVariables.push_back(Var);
>>>>>> + // The first mention of a function argument gets the
>>>>>> FunctionBeginSym
>>>>>> + // label, so arguments are visible when breaking at
>>>>>> function entry.
>>>>>> + DIVariable DV(Var);
>>>>>> + if (DV.Verify() && DV.getTag() ==
>>>>>> dwarf::DW_TAG_arg_variable &&
>>>>>> + DISubprogram(getDISubprogram(DV.getContext()))
>>>>>> + .describes(MF->getFunction()))
>>>>>> + LabelsBeforeInsn[MI] = FunctionBeginSym;
>>>>>> + } else {
>>>>>> + // We have seen this variable before. Try to coalesce
>>>>>> DBG_VALUEs.
>>>>>> + const MachineInstr *Prev = History.back();
>>>>>> + if (Prev->isDebugValue()) {
>>>>>> + // Coalesce identical entries at the end of History.
>>>>>> + if (History.size() >= 2 &&
>>>>>> + Prev->isIdenticalTo(History[History.size() - 2]))
>>>>>> + History.pop_back();
>>>>>> +
>>>>>> + // Terminate old register assignments that don't reach
>>>>>> MI;
>>>>>> + MachineFunction::const_iterator PrevMBB =
>>>>>> Prev->getParent();
>>>>>> + if (PrevMBB != I && (!AtBlockEntry || llvm::next(PrevMBB)
>>>>>> != I) &&
>>>>>> + isDbgValueInDefinedReg(Prev)) {
>>>>>> + // Previous register assignment needs to terminate at
>>>>>> the end of
>>>>>> + // its basic block.
>>>>>> + MachineBasicBlock::const_iterator LastMI =
>>>>>> + PrevMBB->getLastNonDebugInstr();
>>>>>> + if (LastMI == PrevMBB->end())
>>>>>> + // Drop DBG_VALUE for empty range.
>>>>>> + History.pop_back();
>>>>>> + else {
>>>>>> + // Terminate after LastMI.
>>>>>> + History.push_back(LastMI);
>>>>>> + }
>>>>>> + }
>>>>>> + }
>>>>>> + }
>>>>>> + History.push_back(MI);
>>>>>> } else {
>>>>>> + // Not a DBG_VALUE instruction.
>>>>>> + if (!MI->isLabel())
>>>>>> + AtBlockEntry = false;
>>>>>> +
>>>>>> // Check if the instruction clobbers any registers with debug
>>>>>> vars.
>>>>>> for (MachineInstr::const_mop_iterator MOI =
>>>>>> MI->operands_begin(),
>>>>>> MOE = MI->operands_end(); MOI != MOE; ++MOI) {
>>>>>> @@ -2970,19 +3002,57 @@
>>>>>> LiveUserVar[Reg] = 0;
>>>>>>
>>>>>> // Was MD last defined by a DBG_VALUE referring to Reg?
>>>>>> - const MachineInstr *Last = LastDbgValue.lookup(Var);
>>>>>> - if (!Last || Last->getParent() != MI->getParent())
>>>>>> + DbgValueHistoryMap::iterator HistI = DbgValues.find(Var);
>>>>>> + if (HistI == DbgValues.end())
>>>>>> continue;
>>>>>> - if (!isDbgValueInDefinedReg(Last) ||
>>>>>> - Last->getOperand(0).getReg() != Reg)
>>>>>> + SmallVectorImpl<const MachineInstr*> &History =
>>>>>> HistI->second;
>>>>>> + if (History.empty())
>>>>>> continue;
>>>>>> - // MD is clobbered. Make sure the next instruction gets a
>>>>>> label.
>>>>>> - InsnsNeedsLabelAfter.insert(MI);
>>>>>> - RegClobberInsn[Last] = MI;
>>>>>> + const MachineInstr *Prev = History.back();
>>>>>> + // Sanity-check: Register assignments are terminated at
>>>>>> the end of
>>>>>> + // their block.
>>>>>> + if (!Prev->isDebugValue() || Prev->getParent() !=
>>>>>> MI->getParent())
>>>>>> + continue;
>>>>>> + // Is the variable still in Reg?
>>>>>> + if (!isDbgValueInDefinedReg(Prev) ||
>>>>>> + Prev->getOperand(0).getReg() != Reg)
>>>>>> + continue;
>>>>>> + // Var is clobbered. Make sure the next instruction gets
>>>>>> a label.
>>>>>> + History.push_back(MI);
>>>>>> }
>>>>>> }
>>>>>> }
>>>>>> }
>>>>>> + }
>>>>>> +
>>>>>> + for (DbgValueHistoryMap::iterator I = DbgValues.begin(), E =
>>>>>> DbgValues.end();
>>>>>> + I != E; ++I) {
>>>>>> + SmallVectorImpl<const MachineInstr*> &History = I->second;
>>>>>> + if (History.empty())
>>>>>> + continue;
>>>>>> +
>>>>>> + // Make sure the final register assignments are terminated.
>>>>>> + const MachineInstr *Prev = History.back();
>>>>>> + if (Prev->isDebugValue() && isDbgValueInDefinedReg(Prev)) {
>>>>>> + const MachineBasicBlock *PrevMBB = Prev->getParent();
>>>>>> + MachineBasicBlock::const_iterator LastMI =
>>>>>> PrevMBB->getLastNonDebugInstr();
>>>>>> + if (LastMI == PrevMBB->end())
>>>>>> + // Drop DBG_VALUE for empty range.
>>>>>> + History.pop_back();
>>>>>> + else {
>>>>>> + // Terminate after LastMI.
>>>>>> + History.push_back(LastMI);
>>>>>> + }
>>>>>> + }
>>>>>> + // Request labels for the full history.
>>>>>> + for (unsigned i = 0, e = History.size(); i != e; ++i) {
>>>>>> + const MachineInstr *MI = History[i];
>>>>>> + if (MI->isDebugValue())
>>>>>> + requestLabelBeforeInsn(MI);
>>>>>> + else
>>>>>> + requestLabelAfterInsn(MI);
>>>>>> + }
>>>>>> + }
>>>>>>
>>>>>> PrevInstLoc = DebugLoc();
>>>>>> PrevLabel = FunctionBeginSym;
>>>>>> @@ -3043,13 +3113,12 @@
>>>>>> // Clear debug info
>>>>>> CurrentFnDbgScope = NULL;
>>>>>> CurrentFnArguments.clear();
>>>>>> - InsnNeedsLabel.clear();
>>>>>> DbgVariableToFrameIndexMap.clear();
>>>>>> VarToAbstractVarMap.clear();
>>>>>> DbgVariableToDbgInstMap.clear();
>>>>>> DeleteContainerSeconds(DbgScopeMap);
>>>>>> - InsnsNeedsLabelAfter.clear();
>>>>>> - RegClobberInsn.clear();
>>>>>> + UserVariables.clear();
>>>>>> + DbgValues.clear();
>>>>>> ConcreteScopes.clear();
>>>>>> DeleteContainerSeconds(AbstractScopes);
>>>>>> AbstractScopesList.clear();
>>>>>>
>>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=128327&r1=128326&r2=128327&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
>>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Fri Mar 25 21:19:36
>>>>>> 2011
>>>>>> @@ -218,19 +218,16 @@
>>>>>> /// instruction.
>>>>>> DenseMap<const MachineInstr *, MCSymbol *> LabelsAfterInsn;
>>>>>>
>>>>>> - /// insnNeedsLabel - Collection of instructions that need a label
>>>>>> to mark
>>>>>> - /// a debuggging information entity.
>>>>>> - SmallPtrSet<const MachineInstr *, 8> InsnNeedsLabel;
>>>>>> -
>>>>>> - /// InsnsNeedsLabelAfter - Collection of instructions that need a
>>>>>> label after
>>>>>> - /// the instruction because they end a scope of clobber a register.
>>>>>> - SmallPtrSet<const MachineInstr *, 8> InsnsNeedsLabelAfter;
>>>>>> -
>>>>>> - /// RegClobberInsn - For each DBG_VALUE instruction referring to a
>>>>>> register
>>>>>> - /// that is clobbered before the variable gets a new DBG_VALUE, map
>>>>>> the
>>>>>> - /// instruction that clobbered the register. This instruction will
>>>>>> also be in
>>>>>> - /// InsnsNeedsLabelAfter.
>>>>>> - DenseMap<const MachineInstr *, const MachineInstr *>
>>>>>> RegClobberInsn;
>>>>>> + /// UserVariables - Every user variable mentioned by a DBG_VALUE
>>>>>> instruction
>>>>>> + /// in order of appearance.
>>>>>> + SmallVector<const MDNode*, 8> UserVariables;
>>>>>> +
>>>>>> + /// DbgValues - For each user variable, keep a list of DBG_VALUE
>>>>>> + /// instructions in order. The list can also contain normal
>>>>>> instructions that
>>>>>> + /// clobber the previous DBG_VALUE.
>>>>>> + typedef DenseMap<const MDNode*, SmallVector<const MachineInstr*, 4>
>>>>>> >
>>>>>> + DbgValueHistoryMap;
>>>>>> + DbgValueHistoryMap DbgValues;
>>>>>>
>>>>>> SmallVector<const MCSymbol *, 8> DebugRangeSymbols;
>>>>>>
>>>>>> @@ -570,6 +567,23 @@
>>>>>> /// side table maintained by MMI.
>>>>>> void collectVariableInfoFromMMITable(const MachineFunction * MF,
>>>>>> SmallPtrSet<const MDNode *,
>>>>>> 16> &P);
>>>>>> +
>>>>>> + /// requestLabelBeforeInsn - Ensure that a label will be emitted
>>>>>> before MI.
>>>>>> + void requestLabelBeforeInsn(const MachineInstr *MI) {
>>>>>> + LabelsBeforeInsn.insert(std::make_pair(MI, (MCSymbol*)0));
>>>>>> + }
>>>>>> +
>>>>>> + /// getLabelBeforeInsn - Return Label preceding the instruction.
>>>>>> + const MCSymbol *getLabelBeforeInsn(const MachineInstr *MI);
>>>>>> +
>>>>>> + /// requestLabelAfterInsn - Ensure that a label will be emitted
>>>>>> after MI.
>>>>>> + void requestLabelAfterInsn(const MachineInstr *MI) {
>>>>>> + LabelsAfterInsn.insert(std::make_pair(MI, (MCSymbol*)0));
>>>>>> + }
>>>>>> +
>>>>>> + /// getLabelAfterInsn - Return Label immediately following the
>>>>>> instruction.
>>>>>> + const MCSymbol *getLabelAfterInsn(const MachineInstr *MI);
>>>>>> +
>>>>>> public:
>>>>>>
>>>>>> //===--------------------------------------------------------------------===//
>>>>>> // Main entry points.
>>>>>> @@ -593,12 +607,6 @@
>>>>>> ///
>>>>>> void endFunction(const MachineFunction *MF);
>>>>>>
>>>>>> - /// getLabelBeforeInsn - Return Label preceding the instruction.
>>>>>> - const MCSymbol *getLabelBeforeInsn(const MachineInstr *MI);
>>>>>> -
>>>>>> - /// getLabelAfterInsn - Return Label immediately following the
>>>>>> instruction.
>>>>>> - const MCSymbol *getLabelAfterInsn(const MachineInstr *MI);
>>>>>> -
>>>>>> /// beginInstruction - Process beginning of an instruction.
>>>>>> void beginInstruction(const MachineInstr *MI);
>>>>>>
>>>>>>
>>>>>> Modified: llvm/trunk/test/CodeGen/X86/dbg-merge-loc-entry.ll
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/dbg-merge-loc-entry.ll?rev=128327&r1=128326&r2=128327&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/test/CodeGen/X86/dbg-merge-loc-entry.ll (original)
>>>>>> +++ llvm/trunk/test/CodeGen/X86/dbg-merge-loc-entry.ll Fri Mar 25
>>>>>> 21:19:36 2011
>>>>>> @@ -4,7 +4,7 @@
>>>>>>
>>>>>> ;CHECK: Ldebug_loc0:
>>>>>> ;CHECK-NEXT: .quad Lfunc_begin0
>>>>>> -;CHECK-NEXT: .quad Lfunc_end0
>>>>>> +;CHECK-NEXT: .quad L
>>>>>> ;CHECK-NEXT: .short 1 ## Loc expr size
>>>>>> ;CHECK-NEXT: .byte 85 ## DW_OP_reg5
>>>>>> ;CHECK-NEXT: .quad 0
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>
>>>>>
>>>>
>>>
>>
>
>
>
> --
> Alexey Samsonov, MSK
More information about the llvm-commits
mailing list