[PATCH] D98614: [AMDGPU] Fix shortfalls in WQM marking

[Carl Ritson via Phabricator via llvm-commits]

critson added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp:349
   LaneBitmask DefinedLanes;
-  LaneBitmask UseLanes;
-  if (SubReg) {
-    UseLanes = TRI->getSubRegIndexLaneMask(SubReg);
-  } else if (Reg.isVirtual()) {
-    UseLanes = MRI->getMaxLaneMaskForVReg(Reg);
-  }
-
-  SmallPtrSet<const VNInfo *, 4> Visited;
-  SmallVector<const VNInfo *, 4> ToProcess;
-  ToProcess.push_back(UseLRQ.valueIn());
+  unsigned NextPredIdx;
   do {
----------------
piotr wrote:
> I think it would be clearer if you mentioned that NextPredIdx is only needed for the phi case, as opposed to DefinedLanes for instance. Can you please add a comment or change its name to include "phi" somehow (whatever seems right for you)?
Yep, I can rename this variable.


================
Comment at: llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp:368-370
+      for (; PI != PE && !NextValue; ++PI, ++Idx) {
+        if (Idx < NextPredIdx)
+          continue;
----------------
piotr wrote:
> Instead of starting with Idx = 0 and skipping, can you start with Idx = NextPredIdx? That would remove the need for the "continue".
What is happening here is we are skipping through the list of predecessor blocks to the correct number.  I do not think there is a way to access a specific predecessor by index?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98614