[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
Alexey Samsonov
samsonov at google.com
Tue Dec 3 08:27:37 PST 2013
On Tue, Dec 3, 2013 at 11:16 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 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)
>
Don't know, we might. In ASan we build all code with use-after-return
detection support:
define i32 @_Z7my_funci(i32 %a) #0 {
entry:
%MyAlloca = alloca [224 x i8], align 32
//<--------------------------------------- actual stack frame
%0 = ptrtoint [224 x i8]* %MyAlloca to i64
%1 = load i32* @__asan_option_detect_stack_use_after_return
%2 = icmp ne i32 %1, 0
br i1 %2, label %3, label %5
; <label>:3 ; preds = %entry
%4 = call i64 @__asan_stack_malloc_2(i64 224, i64 %0) <-------------
"fake stack frame" returned by ASan runtime
br label %5
; <label>:5 ; preds = %entry, %3
%6 = phi i64 [ %0, %entry ], [ %4, %3 ] <------------------------------
pointer to actual or fake stack frame, where all the local variables are
located.
<...>
%9 = add i64 %6, 96
%10 = inttoptr i64 %9 to i32*
<--------------------------------------------- user variable, given as
offset into either actual or fake stack frame
<...>
call void @llvm.dbg.declare(metadata !{i32* %10}, metadata !18)
I can imaging the situation where "%6" won't be spilled on stack, and we'll
have to report variable location
as "reg storing %6 + offset" (I didn't verify this happens, though).
>
> 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.
>
I don't understand what liveness information you mean here. Looking at
MBB's, we know
which instruction clobbers which (physical) registers, sets of live-ins for
MBBs,
and we know registers each dbg_value instruction refers to. Looks like this
is the granularity
we need - location ranges for each variable in general start and end at
some instructions in the
middle of machine basic blocks. That is, I can't imagine what kind of data
we
may need from register allocator that would simplify the location range
building.
Alternatively, we may move calculation of these ranges and insertion of
location-range-terminatiors earlier, but IMO it's easier to this in
AsmPrinter - after all location
ranges rely on labels, so we don't want to do any transformation of
MachineFunction once we
calculated location ranges for variables.
>
> 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).
>
IIUC, you don't like the following situation happening now:
MOV64mr %RSP, 1, %noreg, 48, %noreg, %R8<kill>; mem:ST8[FixedStack6]
DBG_VALUE %RSP, 48, !"c"; line no:2
Why build a dbg_value and location range for the address "c" (deref($rsp +
48)), if this
address is spilled into a slot with frame index 6? Maybe it's possible to
avoid this,
but this means we'll have to dive into CodeGen, find the place where it
generates
mentioned dbg_value instruction, and instead of generating tell that
location of "c" can
be found using the given stack slot. Or, we should extend dbg_value
instruction and allow it to reference
stack slots, not only registers. I have no idea how to do either of this :(
Also, what should we do if location of variable doesn't refer to $rsp or if
it changes in the middle
of function?
>
> 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
>
--
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131203/a0904226/attachment.html>
More information about the llvm-commits
mailing list