<div dir="ltr"><div>Hi Jay,<br></div><div><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Mar 27, 2021 at 9:56 AM Jay Foad <<a href="mailto:jay.foad@gmail.com">jay.foad@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">For simplicity our regbankselect says that all operands of VALU<br>
instructions have to go in vgprs. Moving some of them into sgprs is<br>
left as an optimisation for a later pass. As you know there are limits<br>
on //how many// operands of a VALU instruction can be sgprs or<br>
constants, which are not simple to express in terms of alternative<br>
operand mappings.<br></blockquote><div><br></div><div>Okay, that makes sense: the initial register bank selection is biased to generate legal code.</div><div><br></div><div>It sounds likely that similar issues of obstructive COPYs will appear in other patterns as well, right? For example, if you have original code of the form (x * (-y)), where x is divergent and y is uniform, it seems likely you'd get something like (typed completely off the top of my head):</div><div><br></div><div>  t:sgpr = G_FNEG y:sgpr</div><div>  t':vgpr = COPY t:sgpr</div><div>  r:vgpr = G_FMUL x:vgpt, t':vgpr<br></div><div><br></div><div>... and then an instruction selection pattern that pulls the negation into a source modifier would fail due to the COPY? The point is that this is not just an issue for constants, so a constant-specific pass seems the wrong way to go.</div><div><br></div><div>The remaining options I see right now are:</div><div><br></div><div>(1) Allow pattern matching to look through COPYs.</div><div>(2) Eliminate those COPYs before instruction selection.<br></div><div><br></div><div>Not having thought about this very long, the relative merits seem to be:</div><div><br></div><div>- (2) results in smaller GMIR and a simpler pattern matcher, which is better for compile time</div><div>- (2) requires legalization after(?) instruction selection (constant bus limit etc.), which is going to be a pain in the existing CodeGen framework<br></div><div><br></div><div>Neither option makes me happy :)  I guess I would go with (2) if this was a greenfield development, but (1) fits better in the existing LLVM code base.</div><div><br></div><div>Cheers,</div><div>Nicolai<br></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Thanks,<br>
Jay.<br>
<br>
On Sat, 27 Mar 2021 at 07:58, Nicolai Hähnle <<a href="mailto:nhaehnle@gmail.com" target="_blank">nhaehnle@gmail.com</a>> wrote:<br>
><br>
> Hi Jay,<br>
><br>
> Where does the copy of a constant from SGPR to VGPR come from? Naively, I'd think that better GMIR would be<br>
><br>
>     %3:sgpr(s32) = G_CONSTANT i32 5<br>
>     ...<br>
>     %8:vgpr(s64) = G_LSHR %7, %3(s32)<br>
><br>
> Cheers,<br>
> Nicolai<br>
><br>
> On Fri, Mar 26, 2021 at 3:43 PM Jay Foad via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>><br>
>> Hi,<br>
>><br>
>> While experimenting with alternative ways of lowering funnel shifts on<br>
>> AMDGPU I have run into a case where this GMIR:<br>
>><br>
>>     %0:vgpr(s32) = COPY $vgpr0<br>
>>     %1:vgpr(s32) = COPY $vgpr1<br>
>>     %2:sgpr_64 = COPY $sgpr30_sgpr31<br>
>>     %3:sgpr(s32) = G_CONSTANT i32 5<br>
>>     %7:vgpr(s64) = G_MERGE_VALUES %1(s32), %0(s32)<br>
>>     %9:vgpr(s32) = COPY %3(s32)<br>
>>     %8:vgpr(s64) = G_LSHR %7, %9(s32)<br>
>>     %4:vgpr(s32) = G_TRUNC %8(s64)<br>
>>     $vgpr0 = COPY %4(s32)<br>
>>     %5:ccr_sgpr_64 = COPY %2<br>
>>     S_SETPC_B64_return %5, implicit $vgpr0<br>
>><br>
>> does not get matched by this selection pattern:<br>
>><br>
>> def : GCNPat<(i32 (trunc (srl i64:$src0, (i32 ShiftAmt32Imm:$src1)))),<br>
>>           (V_ALIGNBIT_B32_e64 (i32 (EXTRACT_SUBREG VReg_64:$src0, sub1)),<br>
>>                           (i32 (EXTRACT_SUBREG VReg_64:$src0, sub0)),<br>
>> VSrc_b32:$src1)>;<br>
>><br>
>> because the COPY of G_CONSTANT 5 from sgpr to vgpr gets in the way of<br>
>> matching the (i32 ShiftAmt32Imm:$src1).<br>
>><br>
>> What's a good way of fixing this? I know there has been some<br>
>> discussion before about whether the generated matcher should look<br>
>> through copies, either automatically or only when the tablegen pattern<br>
>> is augmented with some kind of "please look through copies here" node.<br>
>><br>
>> As an alternative, would it make sense to run some cross-bank constant<br>
>> propagation after regbankselect? It seems pretty obvious to me that %9<br>
>> above should be simplified to:<br>
>><br>
>> %9:vgpr(s32) = G_CONSTANT i32 5<br>
>><br>
>> so long as it's legal, which we can easily check, can't we?<br>
>><br>
>> Thanks,<br>
>> Jay.<br>
>> _______________________________________________<br>
>> LLVM Developers mailing list<br>
>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
><br>
><br>
><br>
> --<br>
> Lerne, wie die Welt wirklich ist,<br>
> aber vergiss niemals, wie sie sein sollte.<br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature">Lerne, wie die Welt wirklich ist,<br>aber vergiss niemals, wie sie sein sollte.</div></div>