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

[Mikael Holmén via llvm-dev]

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.

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?
>