[PATCH] D134641: [AMDGPU][Backend] Fix user-after-free in AMDGPUReleaseVGPRs::isLastVGPRUseVMEMStore

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUReleaseVGPRs.cpp:86
+                                      // used in this block.
+                                      return isLastVGPRUseVMEMStore(*Parent);
+                                    });
----------------
Can't you just inspect the bit vector if these are precomputed without the recursive call?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUReleaseVGPRs.cpp:70
+      BlockVMEMStore.resize(MF.getNumBlockIDs());
+      for (auto &MBB : MF) {
+        BlockVMEMStore[MBB.getNumber()] = lastBlockVGPRUseIsVMEMStore(MBB);
----------------
jmmartinez wrote:
> arsenm wrote:
> > I'd expect this to be the post_order loop
> This loop sets the initialization value for each `MachineBasicBlock`. This loop does not take into account the predecessors/successors and it can be done in any order. Maybe the name `lastBlockVGPRUseIsVMEMStore` is misleading? Could be renamed to something such as `lastVGPRUseInstInBlockIsVMEMStore` or `initVGPRUseInstInBlockIsVMEMStore`.
> 
> The loop after this one is the one that propagates the values from the predecessors to every block and that must be done in reverse-post-order.
Maybe I'm confusing my iteration orders again, but I think you can get away with the one post_order (not reverse) loop if you're looking at successors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134641