[PATCH] D124246: [AMDGPU] Adjust wave priority based on VMEM instructions to avoid duty-cycling.

[Ivan Kosarev via Phabricator via llvm-commits]

kosarev added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSetWavePriority.cpp:42
+
+class MBBInfoSet {
+public:
----------------
foad wrote:
> kosarev wrote:
> > foad wrote:
> > > This seems like overkill. Can you just use a `DenseMap<const MachineBasicBlock *, bool>` directly? Or maybe just a `DenseSet<const MachineBasicBlock *>`?
> > On the other hand, if we know we will need more fields there to count VALUs, then it's probably not worth to rewrite it back and forth? (I don't mind either way.)
> > if we know we will need more fields there to count VALUs
> 
> I don't know that :)
> 
> In any case, why do you use a map of pointers to MBBInfo, instead of just a map of MBBInfo? That seems wasteful.
I think the main desire was to have references to `MBBInfo` remain valid on insertions but also avoid the risk of `DenseMap` be dealing with what may potentially become values of larger size. Will give it another thought on Monday. : )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124246