Page MenuHomePhabricator

[RISCV] Add support for matching vwmulsu from fixed vectors.
ClosedPublic

Authored by Chenbing.Zheng on Jan 25 2022, 11:43 PM.

Details

Summary

According to riscv-v-spec-1.0, widening signed(vs2)-unsigned integer multiply
vwmulsu.vv vd, vs2, vs1, vm # vector-vector
vwmulsu.vx vd, vs2, rs1, vm # vector-scalar

It is worth noting that signed op is only for vs2.
For vwmulsu.vv, we can swap two ops, and don't care which is sign extension,
but for vwmulsu.vx signExt can not be a vector extended from scalar (rs1).
I specifically added two functions ending with _swap in the test case.

Diff Detail

Event Timeline

Chenbing.Zheng requested review of this revision.Jan 25 2022, 11:43 PM
craig.topper added inline comments.Jan 25 2022, 11:54 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
7313

i'm not sure what Op0.getOperand(0).getOperand(1) is.

Op0 is the RISCVISD::VSEXT_VL
Op0.getOperand(0) is the input to the RISCVISD::VSEXT_VL
Op0.getOperand(0).getOperand(1) is the input to the input that, but we don't even know the opcode or how many inputs it has.

But I don't know why we need to check anything special here.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
7313

For this case, I think we couldn't combine it to vwmulsu.vx v8, v9, a1, because vwmulsu.vx signed op is only for v9 here. So I check some specil opcode here , which generate during the process of build_vec from scalar.

define <2 x i16> @vwmulsu_vx_v2i16_swap(<2 x i8>* %x, i8 %y) {

%a = load <2 x i8>, <2 x i8>* %x
%b = insertelement <2 x i8> undef, i8 %y, i32 0
%c = shufflevector <2 x i8> %b, <2 x i8> undef, <2 x i32> zeroinitializer
%d = zext <2 x i8> %a to <2 x i16>
%e = sext <2 x i8> %c to <2 x i16>  // here sext for %c it is build from %y
%f = mul <2 x i16> %d, %e
ret <2 x i16> %f

}

craig.topper added inline comments.Jan 26 2022, 12:18 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
231

Remove , [SDNPCommutative] that's your real bug.

  1. Remove , [SDNPCommutative]
  2. rewrite some code
Chenbing.Zheng marked an inline comment as done.Jan 26 2022, 12:59 AM
Chenbing.Zheng added inline comments.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
7313

I rewrite here, is it better now?

craig.topper added inline comments.Jan 26 2022, 1:03 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
7310

This code doesn't make sense to me. Is it still needed even after removing the SDNPCommutative flag?

Once you've identified a VSEXT and VZEXT everything should be fine. Tablegen should only be able to select the splat on zero extend operand.

Chenbing.Zheng added inline comments.Jan 26 2022, 1:37 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
7310

I try to delete it after removing the SDNPCommutative flag. "vwmulsu_vx_v2i16_swap" test failed and genetate vwmulsu.vx v8, v9, a1.
This code I add here for blocking combine this stuation in "vwmulsu_vx_v2i16_swap" func.
Is there any other better way to block it ?

Optimized vector-legalized selection DAG: %bb.0 'vwmulsu_vx_v2i16_swap:'
SelectionDAG has 20 nodes:

t0: ch = EntryToken
t4: i32,ch = CopyFromReg t0, Register:i32 %1
          t2: i32,ch = CopyFromReg t0, Register:i32 %0
        t8: v2i8,ch = load<(load (s16) from %ir.x)> t0, t2, undef:i32
      t25: nxv1i8 = insert_subvector undef:nxv1i8, t8, Constant:i32<0>
    t28: nxv1i16 = RISCVISD::VZEXT_VL t25, t27, Constant:i32<2>
        t23: v2i8 = BUILD_VECTOR t4, t4
      t30: nxv1i8 = insert_subvector undef:nxv1i8, t23, Constant:i32<0>
    t31: nxv1i16 = RISCVISD::VSEXT_VL t30, t27, Constant:i32<2>
  t33: nxv1i16 = RISCVISD::MUL_VL t28, t31, t27, Constant:i32<2>
t18: ch,glue = CopyToReg t0, Register:nxv1i16 $v8, t33
t27: nxv1i1 = RISCVISD::VMSET_VL Constant:i32<2>
t19: ch = RISCVISD::RET_FLAG t18, Register:nxv1i16 $v8, t18:1

After combine:

SelectionDAG has 18 nodes:

t0: ch = EntryToken
t4: i32,ch = CopyFromReg t0, Register:i32 %1
      t23: v2i8 = BUILD_VECTOR t4, t4
    t30: nxv1i8 = insert_subvector undef:nxv1i8, t23, Constant:i32<0>
        t2: i32,ch = CopyFromReg t0, Register:i32 %0
      t8: v2i8,ch = load<(load (s16) from %ir.x)> t0, t2, undef:i32
    t25: nxv1i8 = insert_subvector undef:nxv1i8, t8, Constant:i32<0>
    t27: nxv1i1 = RISCVISD::VMSET_VL Constant:i32<2>
  t35: nxv1i16 = RISCVISD::VWMULSU_VL t30, t25, t27, Constant:i32<2>
t18: ch,glue = CopyToReg t0, Register:nxv1i16 $v8, t35
t19: ch = RISCVISD::RET_FLAG t18, Register:nxv1i16 $v8, t18:1
craig.topper added inline comments.Jan 26 2022, 9:01 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
7310

With this code removed I get this

vwmulsu_vx_v2i16_swap:                  # @vwmulsu_vx_v2i16_swap
        .cfi_startproc
# %bb.0:
        vsetivli        zero, 2, e8, mf8, ta, mu
        vle8.v  v9, (a0)
        vmv.v.x v10, a1
        vwmulsu.vv      v8, v10, v9
        ret

That looks correct to me. The splat is done as a separate vmv.v.x.

del useless code and update test

Chenbing.Zheng marked 3 inline comments as done.Jan 26 2022, 10:50 PM
Chenbing.Zheng added inline comments.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
7310

Thanks,you are right ~

This revision is now accepted and ready to land.Jan 27 2022, 9:26 AM
This revision was landed with ongoing or failed builds.Jan 27 2022, 6:33 PM
This revision was automatically updated to reflect the committed changes.