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

krishna chaitanya sankisa via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 7 03:27:51 PST 2022


skc7 added a comment.

In D134423#3911623 <https://reviews.llvm.org/D134423#3911623>, @foad wrote:

> What is the motivation for this patch? Is there a bug? Or an existing test that you are trying to generate better code for?
>
> This is a complicated area because in some cases SIFixSGPRCopies has to know what legalizeOperands is going to do, and legalizeOperands can do complicated things like introducing waterfall loops. In the long term it would be better if legalizeOperands stopped doing that and SelectionDAG produced code that was already correct without legalization (e.g. by generating waterfall loops during selection, or even in CodeGenPrepare if we come up with a way to express them in IR). On the other hand, in the long term, it would be nice to abandon SelectionDAG in favour of GlobalISel.
>
>> It is decided based on the SALU instructions users of result of COPY.
>
> This is OK because we (mostly @alex-t!) have put a lot of effort into ensuring that SALU instructions are only selected for uniform operations.
>
>> It misses the case where the use of result of COPY need to be scalar register only. Example: In buffer instructions, there are scalar operands (srsrc, sOffset) which will only accept scalar registers.
>
> This is probably not OK because SelectionDAG still selects buffer instruction where the "scalar operand" is actually a divergent value. It relies on legalizeOperands to fix this by inserting a waterfall loop.

%8:sreg_32 = COPY %5:vgpr_32 
%7:vgpr_32 = BUFFER_LOAD_DWORD_OFFEN %4:vgpr_32, killed %6:sgpr_128, **%8:sreg_32**, 0, 0, 0, 0, implicit $exec ::

si-fix-sgpr-copies pass converts this as below:

%7:vgpr_32 = BUFFER_LOAD_DWORD_OFFEN %4:vgpr_32, killed %6:sgpr_128, **%5:vgpr_32**, 0, 0, 0, 0, implicit $exec :: (dereferenceable load (s32), align 1, addrspace 4)

soffset which is supposed to be scalar register has been converted to use vgpr. This is failing in the backend with error "**Illegal virtual register for instruction**".

Currently, we are seeing such VGPR to SGPR copies and SIFixSGPRCopies pass is using a vgpr for soffset of MBUF. 
I'm trying to fix this issue by checking that scalar registers are used for soffset and srsrc of MUBUF/MTBUF. True, if SelectionDAG already produced code that was legal, that would not require any such fixes.


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

https://reviews.llvm.org/D134423



More information about the llvm-commits mailing list