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

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:11916-11918
+      // CopyToReg may be writing a constant to a sgpr as part of a calling
+      // convention. If that constant is selected to a vgpr then we later need
+      // to copy it into a sgpr. Instead, special case the copying-to-sgpr here to
----------------
arsenm wrote:
> Can you teach isVGPRImm to deal with this
I don't think so.

The failing case here is where a single constant node is needed in a vgpr and in a sgpr. isVGPRImm would need to return true for one case and false for the other, but it takes the node that was CSE'd together for both.

isVGPRImm will currently return false on isSGPRClass. Guessing slightly, the idea behind isVGPRImm is to prefer to materialise in a vgpr directly? Seems reasonable if so, but interacts badly with having no general means of copying from vgpr to sgpr.

I would guess the general case involves a patch to SIFixSGPRCopies.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139852