This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Define vector compare intrinsics.
ClosedPublic

Authored by HsiangKai on Dec 15 2020, 9:34 PM.

Details

Summary

Define vector compare intrinsics and lower them 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: Hsiangkai Wang <kai.wang@sifive.com>

Diff Detail

Event Timeline

HsiangKai created this revision.Dec 15 2020, 9:34 PM
HsiangKai requested review of this revision.Dec 15 2020, 9:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 15 2020, 9:34 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
craig.topper added inline comments.Dec 16 2020, 11:39 AM
llvm/include/llvm/IR/IntrinsicsRISCV.td
220

Can we just have a new class? Especially after my proposal in D93409

Rebase on master.

craig.topper added inline comments.Dec 16 2020, 10:41 PM
llvm/test/CodeGen/RISCV/rvv/vmfeq-rv32.ll
15

Shouldn't we be able to define this so we don't need nvx1i1 in the name?

craig.topper added inline comments.Dec 17 2020, 1:01 AM
llvm/test/CodeGen/RISCV/rvv/vmfeq-rv32.ll
15

Can we add new class to the intrinsics file that uses LLVMScalarOrSameVectorWidth<0, llvm_i1_ty> as the result type? So the mask type is implied without needing to be listed or passed to getDeclaration.

Address @craig.topper's comments.

craig.topper added inline comments.Dec 18 2020, 12:51 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
774

Is it safe for LMUL=1, 1/2, 1/4, 1/8? Does this criteria from the 1.0 spec only when overlap is ok apply

"The destination EEW is smaller than the source EEW and the overlap is in the lowest-numbered part of the source register group (e.g., when LMUL=1, vnsrl.wi v0, v0, 3 is legal, but a destination of v1 is not)."

For LMUL the source register group is always a single register. So does that mean any overlap would always be in the lowest numbered part?

HsiangKai added inline comments.Dec 19 2020, 4:27 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
774

From another statement in the same section.

"The destination EEW is greater than the source EEW, the source EMUL is at least 1, and the overlap is in the highest-numbered part of the destination register group (e.g., when LMUL=8, vzext.vf4 v0, v6 is legal, but a source of v0, v2, or v4 is not)."

It is said the source EMUL is at least 1. There is no such constraint when the destination EEW is smaller than the source EEW. I think it is safe for LMUL = 1, 1/2, 1/4, 1/8.

The earlyclobber constraint is too strong for the overlap rules. Regardless fractional LMUL or not, it is not permitted to overlap the lowest numbered part in the implementation.

Could we use a sequence like this for the tests to make this patch not need D93364?

  1. (a < b) && (b < c) in two instructions vmslt.vv v0, va, vb # All body elements written <- uses the unmasked intrinsic to create a mask inside the function vmslt.vv v0, vb, vc, v0.t # Only update at set mask <- uses the masked intrinsic we want to test

Could we use a sequence like this for the tests to make this patch not need D93364?

  1. (a < b) && (b < c) in two instructions vmslt.vv v0, va, vb # All body elements written <- uses the unmasked intrinsic to create a mask inside the function vmslt.vv v0, vb, vc, v0.t # Only update at set mask <- uses the masked intrinsic we want to test

I will do it to break the dependency.

HsiangKai updated this revision to Diff 313013.Dec 20 2020, 7:52 PM

Update test cases to remove the dependency of D93364.

This revision is now accepted and ready to land.Dec 21 2020, 12:34 PM
This revision was landed with ongoing or failed builds.Dec 21 2020, 10:31 PM
This revision was automatically updated to reflect the committed changes.