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

[Juan Manuel Martinez Caamaño via Phabricator via llvm-commits]

jmmartinez marked an inline comment as done.
jmmartinez added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUReleaseVGPRs.cpp:70
+      BlockVMEMStore.resize(MF.getNumBlockIDs());
+      for (auto &MBB : MF) {
+        BlockVMEMStore[MBB.getNumber()] = lastBlockVGPRUseIsVMEMStore(MBB);
----------------
arsenm wrote:
> 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
I brought back the old algorithm with the recusrive call.

I tried several alternatives but I always found a case where the reverse post order missed propagating the values accordingly :S .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134641