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

Juan Manuel Martinez CaamaƱo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 27 01:56:46 PDT 2022


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


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUReleaseVGPRs.cpp:43
   // Used to cache the result of isLastInstructionVMEMStore for each block
   using BlockVMEMStoreType = DenseMap<MachineBasicBlock *, bool>;
   BlockVMEMStoreType BlockVMEMStore;
----------------
arsenm wrote:
> jmmartinez wrote:
> > arsenm wrote:
> > > If you're going to resize to the number of blocks, do you really need to use a DenseMap at all? You could use a bit vector indexed by the block number 
> > It doesn't map direclty into a bit-vector since there are 3 states considered (has-a-vmem-store, does-not-have-vmem-store and not-yet-computed).
> > 
> > What do you think about a `SmallVector<VMEMStore>` with an `enum class UsesVMEMStoreTy { UsesVMEMStore, DoesNotUseVMEMStore, NotYetComputed }` ?
> Can you avoid the NotYetComputed case if you do this in RPO
Way better! Thanks! I changed it to use a `BitVector`, but also I've encapsulated everything in a small class. The associated object is passed as a parameter to `runOnMachineBasicBlock` (instead of using a member variable).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134641



More information about the llvm-commits mailing list