[PATCH] D96980: [amdgpu] Revert agnostic SGPR spill.

[Michael Liao via Phabricator via llvm-commits]

hliao added a comment.

In D96980#2576128 <https://reviews.llvm.org/D96980#2576128>, @ruiling wrote:

> In D96980#2576062 <https://reviews.llvm.org/D96980#2576062>, @arsenm wrote:
>
>> In D96980#2575930 <https://reviews.llvm.org/D96980#2575930>, @ruiling wrote:
>>
>>> In D96980#2574913 <https://reviews.llvm.org/D96980#2574913>, @sebastian-ne wrote:
>>>
>>>> I think this approach fails when exec is zero.
>>>> The v_mov for the save will be a noop, the v_readfirstline for the restore will read lane 0, which contains some unknown value.
>>>
>>> For exec == 0 when reloading, I think the basic block that contains v_readfirstlane will be jumped over, see `SIInsertSkips.cpp` and `hasUnwantedEffectsWhenEXECEmpty()`
>>
>> I'm trying to eliminate SIInsertSkips. Initially, all branches that go over exec changes should insert the skip jump. We then should eliminate them in cases where they aren't needed and the blocks are short.
>
> I realized `SIRemoveShortExecBranches.cpp` has done correctness checks. So removing SIInsertSkips should work if we make sure there will always be a branching instruction for each control flow change.

When exec mask goes to zero, an SGPR spill in that corresponding code path has no effect at all. As our IR is deSSAed from the SSA form. A def always dominates all its uses. An SGPR spill always dominates its reloads. If that SGPR spill has exec mask 0, all its reloads have exec mask 0 as well. It's simply OK to ignore that spill as, semantically, there is no change in the program state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96980