llvm dev - Feb 2015 - [LLVMdev] JumpThreading and @llvm.dbg.declare()

Hi!

I have a small issue with the JumpThreading pass (in opt). As described 
in the code (JumpThreading.cpp), the pass may if certain criteria are 
fulfilled duplicate the content of a block. If the duplicated block 
contain a call to @llvm.dbg.declare() you will end up with invalid code 
as the intrinsic @llvm.dbg.declare() only should occur once for each 
variable.

I think the JumpThreading pass should lower all llvm.dbg.declare() into 
llvm.dbg.value() the same way as the instcombine pass do (see 
InstructionCombining.cpp):

   // Lower dbg.declare intrinsics otherwise their value may be clobbered
   // by instcombiner.
   bool DbgDeclaresChanged = LowerDbgDeclare(F);

Do you agree? Ok to commit?

Regards
/ Karl-Johan Karlsson

On Thu, Feb 5, 2015 at 4:49 AM, Karl-Johan Karlsson
<karl-johan.karlsson at ericsson.com> wrote:
> I have a small issue with the JumpThreading pass (in opt). As described in
> the code (JumpThreading.cpp), the pass may if certain criteria are
fulfilled
> duplicate the content of a block. If the duplicated block contain a call to
> @llvm.dbg.declare() you will end up with invalid code as the intrinsic
> @llvm.dbg.declare() only should occur once for each variable.
>
> I think the JumpThreading pass should lower all llvm.dbg.declare() into
> llvm.dbg.value() the same way as the instcombine pass do (see
> InstructionCombining.cpp):
>
>   // Lower dbg.declare intrinsics otherwise their value may be clobbered
>   // by instcombiner.
>   bool DbgDeclaresChanged = LowerDbgDeclare(F);
>
> Do you agree? Ok to commit?
Did you forget to attach a patch? A patch with a test case showing the
problem you're describing would be great.

Also, patches should generally be sent to the llvm-commits list, not llvmdev.

Thanks,
Hans

On 02/05/2015 08:21 PM, Hans Wennborg wrote:
> On Thu, Feb 5, 2015 at 4:49 AM, Karl-Johan Karlsson
> <karl-johan.karlsson at ericsson.com> wrote:
>> I have a small issue with the JumpThreading pass (in opt). As described
in
>> the code (JumpThreading.cpp), the pass may if certain criteria are
fulfilled
>> duplicate the content of a block. If the duplicated block contain a
call to
>> @llvm.dbg.declare() you will end up with invalid code as the intrinsic
>> @llvm.dbg.declare() only should occur once for each variable.
>>
>> I think the JumpThreading pass should lower all llvm.dbg.declare() into
>> llvm.dbg.value() the same way as the instcombine pass do (see
>> InstructionCombining.cpp):
>>
>>    // Lower dbg.declare intrinsics otherwise their value may be
clobbered
>>    // by instcombiner.
>>    bool DbgDeclaresChanged = LowerDbgDeclare(F);
>>
>> Do you agree? Ok to commit?
>
> Did you forget to attach a patch? A patch with a test case showing the
> problem you're describing would be great.
>
> Also, patches should generally be sent to the llvm-commits list, not
llvmdev.
I will create a patch with a testcase and send it to llvm-commits.

Regards
/ Karl-Johan Karlsson
>
> Thanks,
> Hans
>
>