[PATCH] D95503: [AMDGPU] WQM/WWM: Fix marking of partial definitions

[Carl Ritson via Phabricator via llvm-commits]

critson marked 2 inline comments as done.
critson added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp:265
                                std::vector<WorkItem> &Worklist) {
-  assert(!MRI->isSSA());
-
----------------
arsenm wrote:
> You lost this assert (but we should convert it to the pass required properties set anyway)
Yes, while I think I deleted the assertion by accident, I believe it is technically wrong.
I will add clearedProperties to the pass in D94746 which is where !IsSSA is definitely required.


================
Comment at: llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp:349
 
-    Register Reg = Use.getReg();
+  // Ignore some hardware registers
+  switch (Reg) {
----------------
arsenm wrote:
> Should this skip all implicit operands?
No we must process implicit uses as well and mark them as WWM/WQM with $vcc being the primary use case, e.g. $vcc input of WWM VOP2 v_addc_co must also be in computed in WWM. 
At the moment $exec is the only register I am confident that can be ignored as it explicitly managed by this pass; however, it is possible other register (e.g. $m0 and $mode) might also be valid here, but I'd prefer to leave that as exercise for later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95503