[PATCH] D131246: [AMDGPU] SIFixSGPRCopies refactoring

Alexander via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 14 06:17:28 PDT 2022


alex-t marked an inline comment as done.
alex-t added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:1108
+  if (T != PostProcessingLists.end()) {
+    T->second.List->erase(T->second.I);
+    PostProcessingLists.erase(MI);
----------------
rampitec wrote:
> Why not simply clean all of these lists/vectors/maps at the end of the runOnMachineFunction?
The reason for that change is as follows:
We cannot lower SCC copies, REG_SEQUENCE, and PHIs **//before//** VGPR to SGPR copies since any lowering could change the MIR and, hence, make the analysis results invalid.
On other hand, in VGPR to SGPR copies lowering "SIInstrInfo::moveToVALU & Co" may be invoked. They remove MIs from the basic block and poison the memory allocated for them. So, all the pointers stored in the lists can potentially point to the poisoned memory and MI->getParent() check becomes invalid. This is the exact reason for the ASAN failure.
The "OnDeleteListener" interface serves as the delegate passed to "SIInstrInfo::moveToVALU" to make the caller informed of the MI->eraseFromParent() calls.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131246/new/

https://reviews.llvm.org/D131246



More information about the llvm-commits mailing list