This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Custom lower vector llvm.is.fpclass to vfclass.v
ClosedPublic

Authored by liaolucy on May 22 2023, 8:13 PM.

Details

Summary

After D149063.
This patch adds support for both scalable and fixed-length vector.

Diff Detail

Event Timeline

liaolucy created this revision.May 22 2023, 8:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 22 2023, 8:13 PM
liaolucy requested review of this revision.May 22 2023, 8:13 PM
liaolucy updated this revision to Diff 524556.May 22 2023, 8:18 PM

clang-format

craig.topper added inline comments.May 22 2023, 11:17 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
1046

Isn't VT an FP type here?

4382

Why not scalable vectors?

4398

Capitalize variables names

4400

Shouldn't we be emitting an AND and SETNE with 0 like the scalar code?

craig.topper added inline comments.May 22 2023, 11:19 PM
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-vfclass.ll
12

The result of vfclass can only have single bit set according to the scalar documentation "Note that exactly one bit in rd will be set. FCLASS.S does not set the floating-point exception flags." So it can never equal 768.

craig.topper added inline comments.May 22 2023, 11:21 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
2013

Indent this 2 more spaces so it lines up with (vti.Vector on the previous line.

liaolucy updated this revision to Diff 524975.May 23 2023, 7:27 PM
liaolucy marked 3 inline comments as done.

Update try to address comments.
Use AND and SETNE.
Update test case.

If the changes are correct, then it seems that this patch is not needed:
testcase:

declare <2 x i1> @llvm.is.fpclass.v2f32(<2 x float>, i32)
define <2 x i1> @isnan_v2f32(<2 x float> %x) nounwind {
  %1 = call <2 x i1> @llvm.is.fpclass.v2f32(<2 x float> %x, i32 3)  ; nan
  ret <2 x i1> %1
}

Without this patch:

vsetivli        zero, 2, e32, mf2, ta, ma
vmfne.vv        v8, v8, v8
vmor.mm v0, v8, v8

with the patch, we get:

vsetivli        zero, 2, e32, mf2, ta, ma
vfclass.v       v8, v8
li      a0, 768
vand.vx v8, v8, a0
vmsne.vi        v0, v8, 0
ret
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
4382

I did not find the scalable llvm.is.fpclass.
https://llvm.org/docs/LangRef.html#llvm-is-fpclass-intrinsic
It seems that this scalableintrinsic should be supported first.

Update try to address comments.
Use AND and SETNE.
Update test case.

If the changes are correct, then it seems that this patch is not needed:
testcase:

declare <2 x i1> @llvm.is.fpclass.v2f32(<2 x float>, i32)
define <2 x i1> @isnan_v2f32(<2 x float> %x) nounwind {
  %1 = call <2 x i1> @llvm.is.fpclass.v2f32(<2 x float> %x, i32 3)  ; nan
  ret <2 x i1> %1
}

Without this patch:

vsetivli        zero, 2, e32, mf2, ta, ma
vmfne.vv        v8, v8, v8
vmor.mm v0, v8, v8

with the patch, we get:

vsetivli        zero, 2, e32, mf2, ta, ma
vfclass.v       v8, v8
li      a0, 768
vand.vx v8, v8, a0
vmsne.vi        v0, v8, 0
ret

The default expansion does know to create single instructions for some cases, but will probably generate worse code for mixing classes together. You should add more testing of different combinations of bits. Maybe we want to use fclass for some cases and default expansion for others.

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

I don't think the syntax in the LangRef is an exhaustive list. Have you tried it?

liaolucy updated this revision to Diff 525411.May 24 2023, 7:46 PM
liaolucy retitled this revision from [RISCV] Custom lower fixed-length vector llvm.is.fpclass to vfclass.v to [RISCV] Custom lower vector llvm.is.fpclass to vfclass.v.
liaolucy edited the summary of this revision. (Show Details)
  1. support scalable vector
  2. add more testcases
craig.topper added inline comments.May 24 2023, 8:55 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
4389

Reuse RISCVISD::FPCLASS_VL here. You can pass DAG.getRegister(RISCV::X0, XLenVT) as the VL.

4417

This is a VMSNE

liaolucy updated this revision to Diff 525456.May 24 2023, 11:45 PM

Reuse RISCVISD::FPCLASS_VL
change the value name to VMSNE

This revision is now accepted and ready to land.May 25 2023, 9:12 PM
This revision was landed with ongoing or failed builds.May 25 2023, 11:44 PM
This revision was automatically updated to reflect the committed changes.
craig.topper added inline comments.May 26 2023, 12:01 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
4405

This MVT::Other result looks unnecessary. Removed in 1fea6bd8777a800af8f67e5d67afe25a713a8926