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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D87107#2257077 <https://reviews.llvm.org/D87107#2257077>, @rampitec wrote:

> In D87107#2257000 <https://reviews.llvm.org/D87107#2257000>, @alex-t wrote:
>
>> In D87107#2256913 <https://reviews.llvm.org/D87107#2256913>, @rampitec wrote:
>>
>>> 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.
>>
>> Let's forget about exec == 0...
>> The code snippet above looks for exec restoring instruction in the block prologue and return true if it exists.  So, false positive can only happen if there is no exec restoring instruction in the beginning of the block but it is placed in the middle of the block. In this case I report the block is invalid for splitting despite it is valid to split after the exec restoring instruction in the middle of the block. Do we really have exec restored in the middle of the block?
>
> No, we do not change exec in the middle of the block.
> It would help to understand what are you doing if you would have a test to show.

This does happen, although perhaps it shouldn't (WQM emit these for example)


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

https://reviews.llvm.org/D87107