[PATCH] D69280: [AMDGPU] Allow folding of sgpr to vgpr copy

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec marked an inline comment as done.
rampitec added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/fmul-2-combine-multi-use.ll:79-80
 ; SIVI:  v_mad_f32 {{v[0-9]+}}, |[[X]]|, 2.0, v{{[0-9]+}}
-; GFX10: v_fma_f32 {{v[0-9]+}}, |[[X:s[0-9]+]]|, 2.0, {{s[0-9]+}}
-; GFX10: v_fma_f32 {{v[0-9]+}}, |[[X]]|, 2.0, {{s[0-9]+}}
+; GFX10: v_fma_f32 {{v[0-9]+}}, 2.0, |[[X:s[0-9]+]]|, {{v[0-9]+}}
+; GFX10: v_fma_f32 {{v[0-9]+}}, 2.0, |[[X]]|, {{v[0-9]+}}
 define amdgpu_kernel void @multiple_use_fadd_multi_fmad_f32(float addrspace(1)* %out, float %x, float %y, float %z) #0 {
----------------
rampitec wrote:
> arsenm wrote:
> > rampitec wrote:
> > > rampitec wrote:
> > > > arsenm wrote:
> > > > > This looks like it got worse?
> > > > Yes, this is regression specific to fma/mac. The reg class after the folding mismatches xm0/xexec operand definition of fma src.
> > > > The regression is however small, while some copies are eliminated in other cases.
> > > I.e. we should refine how we use sgpr register classes instead of inhibiting folding.
> > The fma src doesn't use xm0_xexec though? Can you add a testcase with this specific case? I think this should be easily avoidable
> It is explicitly disabled in the SIFoldOperands::foldOperand():
> 
> 
> ```
>     // Don't fold subregister extracts into tied operands, only if it is a full
>     // copy since a subregister use tied to a full register def doesn't really
>     // make sense. e.g. don't fold:
>     //
>     // %1 = COPY %0:sub1
>     // %2<tied3> = V_MAC_{F16, F32} %3, %4, %1<tied0>
>     //
>     //  into
>     // %2<tied3> = V_MAC_{F16, F32} %3, %4, %0:sub1<tied0>
>     if (UseOp.isTied() && OpToFold.getSubReg() != AMDGPU::NoSubRegister)
>       return;
> 
> ```
This code is removed in child D69287 and folding happens again.


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

https://reviews.llvm.org/D69280