This patch fixes an issue observed upon applying 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.
I don't think you should call Deallocate when using a BumpPtrAllocator?
include/llvm/Support/Allocator.h
210 // Bump pointer allocators are expected to never free their storage; and 211 // clients expect pointers to remain valid for non-dereferencing uses even 212 // after deallocation. 213 void Deallocate(const void *Ptr, size_t Size, size_t /*Alignment*/) { 214 __asan_poison_memory_region(Ptr, Size); 215 }IIUC the memory will be freed by the BumpPtrAllocator with everything else all at once.