[PATCH] D134423: [AMDGPU] Fix vgpr2sgpr copy to scalar operands of buffer instructions.

[Alexander via Phabricator via llvm-commits]

alex-t added a comment.

Sorry for my tediousness but
I would like to see any inspirational reason for this change. Namely, what is wrong without it:

- compile time error - example
- incorrect code generated - example
- suboptimal code (Yes I know this really should be one) - example

The change in the LIT test suggests one, but I would like to see (if possible) a simpler case and verbal description regarding "why do we need this?"



================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:922
+            const MachineOperand *soffsetMO =
+                TII->getNamedOperand(U, AMDGPU::OpName::soffset);
+
----------------
skc7 wrote:
> foad wrote:
> > I don't understand why we need special cases for particular named operands. Why can't this be inferred from the operand's register class? @alex-t?
> soffset and srsrc need to be scalar registers in mubuf/mtbuf instructions.. Check for use of COPY's result is done here for these operands.
The fact that the specific use requires SGPR surely can be checked for any use.
What I don't like here is the hack forcing the decision to be made for v_readfistlane.
The algorithm that makes a decision is a complex tradeoff and we should not "tune" it in such a way for each particular case. I would consider the "SGPR required" as a weight value for the solver rather than the ultimate condition.


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

https://reviews.llvm.org/D134423