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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm 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
----------------
JonChesterfield wrote:
> 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.
Correct. False is always a correct answer. Copy and rematerialize in VGPR later is easy


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139852