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

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSetWavePriority.cpp:42
+
+class MBBInfoSet {
+public:
----------------
This seems like overkill. Can you just use a `DenseMap<const MachineBasicBlock *, bool>` directly? Or maybe just a `DenseSet<const MachineBasicBlock *>`?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSetWavePriority.cpp:97
+
+MachineInstr *AMDGPUSetWavePriority::BuildSetprioMI(MachineFunction &MF,
+                                                    unsigned priority) const {
----------------
I think it would be neater to have this function take a MachineBasicBlock::iterator instead of MF, and have it automatically insert the created instruction (by calling one of the BuildMI overloads that does the insertion).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124246