[PATCH] D99423: [DebugInfo] Fix incorrect updating of SDNode dependencies for variadic debug values

Stephen Tozer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 26 09:50:12 PDT 2021


StephenTozer created this revision.
StephenTozer added reviewers: probinson, aprantl, djtodoro, vsk, dblaikie, jmorse, Orlando.
StephenTozer added a project: debug-info.
Herald added a subscriber: hiraditya.
StephenTozer requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This patch fixes an issue observed upon applying D91722 <https://reviews.llvm.org/D91722> to current main (previously contained in another patch, but now separated out here):

SelectionDAG::transferDebugValues incorrectly updates the SDDbgValue's dependencies list. Specifically, this class maintains two closely related lists; the list of SDDbgOperands(which, for nodes, consist of an SDNode and a result number, (Node, ResNo)), and the list of SDNodes that it depends on (this list contains duplicates, which is important here). The latter list is derived from the former (this isn't true for all SDDbgValues, but it is true for all variadic cases), so we can expect each node operand to have a corresponding dependency. When we transfer debug values, currently we replace each operand that matches the (Node, ResNo) with the target node and result number, but we then replace every dependency of Node with the target node as well. This is incorrect if we have operands [(Node1, 0), (Node1, 1)], as the dependencies will be [Node1, Node1], so when we want to replace (Node1, 0) with (Node2, 0), we end up with operands [(Node2, 0), (Node1, 1)] (correct), and dependencies [Node2, Node2] (incorrect).

My current approach to fixing this is to change the dependency handling in SDDbgValue so that we derive the dependencies from the operands where possible. This is done by keeping a list of AdditionalDependencies, which contains all of the debug value's dependencies that aren't used directly as operands. When we fetch the list of dependencies, we fetch a combination of the nodes used by location operands and the additional dependencies; this means that we can freely update the debug operands without worrying about keeping the dependencies array in sync. Technically this approach could be taken further, as all the additional dependencies actually come from frame index location operands, but this kind of tracking isn't handled even for the existing cases; as this is currently holding up the main variadic patch, it seemed prudent to stick to the simple case for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99423

Files:
  llvm/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99423.333574.patch
Type: text/x-patch
Size: 4949 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210326/91f739da/attachment.bin>


More information about the llvm-commits mailing list