[llvm-dev] Jump Threading duplicates dbg.declare intrinsics for fragments, bug?

[Adrian Prantl via llvm-dev]

> On Sep 20, 2017, at 1:43 AM, Mikael Holmén <mikael.holmen at ericsson.com> wrote:
> 
> Hi all,
> 
> Thanks for the answers!
> 
> I feel like I've hijacked your thread now though Björn, sorry for that.
> 
> But from the answers it sounds like there is agreement that it's reasonable to remove the duplicates as done in Björn's patch?
> 
> ---
> 
> A couple of more things around the problem I saw.
> 
> On 09/19/2017 05:40 PM, Adrian Prantl wrote:
> > A dbg.declare describes a stack-allocated variable. There may only be > one dbg.declare per source variable, with the one exception that if
> > the source variable is split up into multiple fragments (such as SROA)
> > there may be one dbg.declare per variable fragment.
> 
> How about this:
> 
>  %b.sroa.4.i = alloca [32 x i32]
>  call void @llvm.dbg.declare(metadata [32 x i32]* %b.sroa.4.i, metadata !10, metadata !DIExpression(DW_OP_LLVM_fragment, 32, 1024)), !dbg !18
>  call void @llvm.dbg.declare(metadata [32 x i32]* %b.sroa.4.i, metadata !10, metadata !DIExpression(DW_OP_LLVM_fragment, 32, 1024)), !dbg !20
> 
> The dbg.declares are identical except that !18 and !20 have different inlinedAt fields. The above is the result of the alloca-merging done by the inliner. After inlining two calls to the same function it merges two allocas and keeps the dbg.declares from each of them.

Note that two debug intrinsics with different inlinedAt fields are like two separate variables for all that matters. I would be surprised if that was related to you problem. (But, I keep getting surprised every day :-)

-- adrian

> 
> On 09/19/2017 05:44 PM, Reid Kleckner wrote:
> > I think it's a bug in both places: the backend should tolerate
> > identical, duplicate dbg.declares, and the loop unroller probably
> > shouldn't duplicate dbg.declare, since there is no point.
> 
> I also think it's weird that the replaceDbgDeclareForAlloca/replaceDbgDeclare/FindAllocaDbgDeclare methods return as soon as they've handled one found dbg.declare, so if there are several, one is moved next to the alloca, but the other ones are left somewhere. So maybe a fault there too? These methods are used from a bunch of different passes...
> 
> Thanks,
> Mikael
> 
> On 09/19/2017 05:46 PM, Adrian Prantl wrote:
>>> On Sep 19, 2017, at 8:44 AM, Reid Kleckner <rnk at google.com <mailto:rnk at google.com>> wrote:
>>> 
>>> On Tue, Sep 19, 2017 at 8:40 AM, Adrian Prantl via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>>> 
>>>    > Later loop unroll comes and unrolls the loop and then suddenly we have two absolutely identical dbg.declares and the assert in addFragmentOffset() blows. Who's at fault?
>>> 
>>>    Without having read the code yet, my intuition says that the
>>>    unroller should not be duplicating dbg.declares, only dbg.values.
>>> 
>>> 
>>> I think it's a bug in both places: the backend should tolerate identical, duplicate dbg.declares,
>> I guess that's fair, yes.
>> -- adrian
>>> and the loop unroller probably shouldn't duplicate dbg.declare, since there is no point.
>>> 
>>> IR is supposed to be duplicatable unless it is marked noduplicate. That was ultimately the fix we applied for PR33157, right?
>