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