Hi Jonas,
Thanks for starting the discussion here. I am going to update the subject line
some people might overlook it otherwise. I have a few thoughts which I have
highlighted below.
From: llvm-dev <llvm-dev-bounces@lists.llvm.org> On Behalf Of Jonas Paulsson
via llvm-dev
Sent: Friday, May 4, 2018 2:57 AMHi,
I recently found myself in trouble because the crash I had disappeared
with -g, so I could not debug the program. This happened because the
optimizer did not remember to consider DBG_VALUEs instruction so it
changed its behavior, and the bug went hiding.I then started discussing this on https://reviews.llvm.org/D45878, and
since this is something that should be handled by all different machine
passes, the immediate question is now what utility functions should be
made available for common use.
[... snip ...]
We are discussing where to place these utility functions, and if
splice() should optionally be made to do this also. It seems that
collectDebugValues() in MachineSink.cpp is a good starting point, but we
probably want to do an even better search than just looking at the first
def operand.It would be nice to get some general feedback from the community at this
point so we know which direction to take...
I'm unbiased to any particular approach. In any case, it does seem helpful
having collectDebugValues() in MachineBasicBlock.h. There are already a few
other utilities in there for ignoring debug and phi instructions, such as
getFirstNonDebugInstr().
You mentioned the idea of extending the logic of splice() to take into account
an instruction's associated debug instructions. If I understand you correctly,
the idea is that splice would move both the original instruction and its
associated DBG_VALUE. I'm not sure that is the correct approach, I'm not sure
it's wrong either. It does solve your problem, but it seems to special case the
idea of an instruction (e.g., regular instruction or regular instruction with
associated debug instruction). If you change splice(), would erase() and other
instruction-motion related routines in MBB change? I assume insert() would
not be updated. As far as I am aware, IR/BasicBlock.h does not special case the
debug intrinsics when it moves instructions around.
Anyways, those are just a few thoughts. Thanks for starting up the discussion Jonas.
-Matt