[PATCH] D87107: [AMDGPU] Target hook to apply target specific split constraint

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D87107#2256864 <https://reviews.llvm.org/D87107#2256864>, @alex-t wrote:

>> Also I still think that disabling a whole "endif" block is an overkill.
>
> It only is disabled if S_OR_B64 exec, ... is in the middle of the block that should never happen.
>
>   while (isBasicBlockPrologue(*J)) {
>     if (IsExecRestore(&*J))
>       return true;
>
> assumes that if exec is restored in the block prologue it is valid

So practically it never happens and split is effectively only disabled in an empty block? I said it already: it does not matter that exec is zero, what matters is that it does not match. It does not matter that a block is empty as well, it is enough to split before s_or to hit the bug.



================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6811
+    return (Instr->modifiesRegister(AMDGPU::EXEC, TRI) &&
+                (Instr->getOpcode() == AMDGPU::S_OR_B64) ||
+            (Instr->getOpcode() == AMDGPU::S_MOV_B64 &&
----------------
Not speaking of the wave32, you cannot rely on the S_OR instruction. It can be restored with a XOR as well.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6817
+    MachineBasicBlock::iterator I = Pred->getFirstTerminator();
+    if (I != Pred->end() && I->modifiesRegister(AMDGPU::EXEC, TRI)) {
+      while (++I != Pred->end()) {
----------------
S_OR shall not be a terminator, it is used in the prologue.


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

https://reviews.llvm.org/D87107