[PATCH] D122797: [AMDGPU] Add missing use check in SIOptimizeExecMasking pass.

[Jay Foad via Phabricator via llvm-commits]

foad accepted this revision.
foad added a comment.
This revision is now accepted and ready to land.

Looks OK as it fixes a bug, but I think the liveness checking could be cleaned up as noted inline.



================
Comment at: llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp:414-422
   if (isLiveOut(*VCmp->getParent(), VCmpDest->getReg()))
     return nullptr;
 
-  // If the v_cmp target is in use after the s_and_saveexec, skip the
-  // optimization.
-  if (isRegisterInUseAfter(SaveExec, VCmpDest->getReg(), TRI, MRI))
+  // If the v_cmp target is in use between v_cmp and s_and_saveexec or after the
+  // s_and_saveexec, skip the optimization.
+  if (isRegisterInUseBetween(*VCmp, SaveExec, VCmpDest->getReg(), TRI, MRI,
+                             false, true) ||
----------------
I'm not sure that the isRegisterInUseBetween and isRegisterInUseAfter helper functions are actually making this code any simpler, or easier to understand.

The conditions you really want to check are:
1. is VCmpDest->getReg() live immediately after SaveExec? You can do this with LivePhysRegs, and this check would replace both the isLiveOut check (line 414) and the isRegisterInUseAfter check.
2. is VCmpDest->getReg() used by an instruction between *VCmp and SaveExec? You don't need any kind of liveness for this check, you can just examine the operands of every instruction in that range.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122797