[PATCH] D27997: [AMDGPU] Prevent spills before exec mask is restored

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D27997#634166, @rampitec wrote:

> In https://reviews.llvm.org/D27997#633402, @MatzeB wrote:
>
> > In general I find rules like "no spill may be before a isPositionLike() instruction" dangerous in that they are not at all obvious to people writing generic codegen passes and you run the danger that even if you fix the register allocator other passes may disregard the new rule.
> >
> > Have you considered alternative solutions? Like having a late pass that moves your exec mask back to the top in a late pass so that the exec mask rules only need to be known by that one pass?
>
>
> I have started with exploring alternative solutions which do not require any target callbacks. That turns to be pretty difficult and dangerous. Not only that is extremely tricky to recover, but also creates some paradoxes, like if I need a register being spilled before the new insertion point. Also instructions which operate exec mask are all the same at the beginning of a BB and at the end, and in case of a small BB that would be too late to distinguish. The safest solution is not to recover from broken IR, but prevent the problem at the first place.
>
> Regarding other passes which might do the same, I'm not aware of them. Machine instructions have all bits like side effects and register use-def dependencies set, so no pass (like LICM for example) can illegally rearrange them. That is in particular difficult to recover from wrong spilling, because I would have to perform formally illegal transformations. Only the regalloc is a problem because it does not try to rearrange instructions, it just inserts new.


Ok so if I understand this correctly the dependencies are actually visible by means of defs/uses of some mask register, this only affects spill placement code just because it is not obvious that a spill instruction will read the mask register and before actually creating it?

I don't like the name `isPositionLike()`. The semantic do not seem to be too similar to `isPosition()` which are instructions that mark a place in a function that is somehow visible elsewhere (because of exception handling tables, ...), this is not the case here. Also if the only use of this function is fixing a spill code placement problem, then the documentation comment should mention that.


Repository:
  rL LLVM

https://reviews.llvm.org/D27997