[PATCH] D69280: [AMDGPU] Allow folding of sgpr to vgpr copy
Stanislav Mekhanoshin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 21 16:06:43 PDT 2019
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 {
----------------
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;
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69280/new/
https://reviews.llvm.org/D69280
More information about the llvm-commits
mailing list