[PATCH] D139852: [amdgpu] Lower CopyToReg into SGPR explicitly to avoid illegal vgpr to sgpr copy

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added a comment.

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

> In D139852#3989299 <https://reviews.llvm.org/D139852#3989299>, @JonChesterfield wrote:
>
>> In D139852#3989251 <https://reviews.llvm.org/D139852#3989251>, @alex-t wrote:
>>
>>> Since we ensure all the VGPR to SGPR copies are uniform, we just need to V_READFIRSTLANE_B32 here.
>>
>> What ensures this copy is uniform?
>
> The divergence-driven ISel does.
> The call argument would be VGPR if it is divergent.

The call argument here isn't a vgpr - it's part of the calling convention for passing implicit parameters around, so it's explicitly a sgpr. We could have defined it to be a vgpr containing a uniform value instead, but as far as I can tell that would be strictly worse.

What we have at present is a constant node gets lowered to an instruction that leaves the result in a vgpr, and then miscompilation. We could indeed lower the constant to a vgpr and then notice that we've done that and insert a readfirstlane to fix it up, instead of erroring, that mostly means updating something downstream of ISel to know that a given vector register is uniform based on the instruction it came from.

I'm not totally convinced by our compiler emitting invalid code and then trying to fix it up later. This is a case where we can emit something which is correct up front, without divergence analysis or regressions. It's also associated with function calls which are quite high complexity as the baseline.

I think the right fix here is

- have globalisel model vector and scalar register files as separate things (if it doesn't already)
- teach regalloc about vector and scalar register files as most of the fixup pass seems to be trying to undo things regalloc did wrong
- delete sdag


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139852