This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Define vslideup/vslidedown intrinsics and lower to V instructions.
ClosedPublic

Authored by arcbbb on Dec 15 2020, 1:33 AM.

Details

Summary

Define vslideup/vslidedown intrinsics and lower to V instructions.

We work with @rogfer01 from BSC to come out this patch.

Authored-by: Roger Ferrer Ibanez <rofirrim@gmail.com>
Co-Authored-by: ShihPo Hung <shihpo.hung@sifive.com>

Diff Detail

Unit TestsFailed

Event Timeline

arcbbb created this revision.Dec 15 2020, 1:33 AM
arcbbb requested review of this revision.Dec 15 2020, 1:33 AM
arcbbb updated this revision to Diff 312084.Dec 15 2020, 6:12 PM

rebase the patch.

The destination register group for vslideup cannot overlap with the source vector register group, so I think we need an extra earlyclobber constraint passed to those pseudos.

craig.topper added a comment.EditedDec 16 2020, 11:16 AM

Please upload patches with git diff -U999999 so that we have full context.

If I'm reading the spec correctly, the offset from the scalar register is not truncated to SEW before applying. So that means the ANY_EXTEND we use in LowerINTRINSIC_WO_CHAIN to extend the operand to XLenVT is incorrect for these intrinsics. We would need a ZERO_EXTEND instead. The C intrinsic spec defines them as taking size_t for offset. We should probably preserve that into the IR intrinsic.

llvm/include/llvm/IR/IntrinsicsRISCV.td
118

Is the slideup/slidedown type always the same as the element type? Can we use LLVMVectorElementType<0> instead of llvm_any_ty?

craig.topper added inline comments.Dec 16 2020, 11:24 AM
llvm/include/llvm/IR/IntrinsicsRISCV.td
118

I can't delete my previous comment. I think llvm_any_ty here should be llvm_anyint_ty and the tests should use i64 for rv64 and i32 for rv32.

craig.topper added inline comments.Dec 16 2020, 11:33 AM
llvm/include/llvm/IR/IntrinsicsRISCV.td
118

And I guess the llvm_anyint_ty for VL can then just be LLVMMatchType<1> since it also needs to be XLen.

arcbbb updated this revision to Diff 312405.Dec 17 2020, 1:35 AM

Updates:

  1. fixed @earlyclobber on vslideup
  2. fixed llvm_any_ty to llvm_anyint_ty, and VL to LLVMMatchType<1>
  3. fixed tests based on above reviews.
  4. rebase to latest master.
  5. use LLVMScalarOrSameVectorWidth for mask type.

Thanks!

craig.topper added inline comments.Dec 17 2020, 10:59 AM
llvm/include/llvm/IR/IntrinsicsRISCV.td
185

Drop ExtendOperand. It shouldn't be needed now.

192

Drop ExtendOperand.

llvm/test/CodeGen/RISCV/rvv/vslidedown-rv32.ll
3

This intrinsic name shouldn't have ".i8". It should be ".i32" or dropped.

9

Like wise the test name shouldn't have _i8

arcbbb updated this revision to Diff 312663.Dec 17 2020, 6:54 PM
  1. rebased
  2. fixed tests based on Craig's comment
craig.topper accepted this revision.Dec 18 2020, 1:49 PM

LGTM

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
1441

Should this be below 14.2?

This revision is now accepted and ready to land.Dec 18 2020, 1:49 PM
This revision was automatically updated to reflect the committed changes.