This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Teach lowerCTLZ_CTTZ_ZERO_UNDEF to handle conversion i32/i64 vectors to f32 vectors.
ClosedPublic

Authored by fakepaper56 on Dec 30 2022, 8:07 AM.

Details

Summary

Previously lowerCTLZ_CTTZ_ZERO_UNDEF converted the source to float value by
ISD::UINT_TO_FP. ISD::UINT_TO_FP uses dynamic rounding mode, so the rounding
may make the exponent of the result not as expected when converting i32/i64 to f32.
This is the reason why we constrained lowerCTLZ_CTTZ_ZERO_UNDEF to only handle
an i32 source when the f64 type having the same element count as source is legal.

The patch teaches lowerCTLZ_CTTZ_ZERO_UNDEF converts i32/i64 vectors to f32
vectors by vfcvt.f.xu.v with RTZ rounding mode. Using RTZ is to make sure the
exponent of results is correct, although f32 could not totally represent each
value in i32/i64.

Diff Detail

Event Timeline

fakepaper56 created this revision.Dec 30 2022, 8:07 AM
fakepaper56 requested review of this revision.Dec 30 2022, 8:07 AM
fakepaper56 added inline comments.Dec 31 2022, 7:23 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
143

I have tried to use similar naming as SDT_RISCVVecCvtX2FOp_VL riscv_vfcvt_rm_x_f_vl used. But I confused that SDT_RISCVVecCvtX2FOp_VL looks like that it's for converting x to f rather than converting f to x.

craig.topper added inline comments.Jan 3 2023, 9:49 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
679–680

This comment needs updating. With this patch we no longer require the floating point type to represent the value exactly.

llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
143

You're right. I did name that badly. I'll fix it.

Update outdated comment.

fakepaper56 marked an inline comment as done.Jan 4 2023, 6:57 AM
fakepaper56 added inline comments.
llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
143

Thank you. If you are busy, I could open a revision to rename SDT_RISCVVecCvtX2FOp_VL to SDT_RISCVVecCvtF2XOp_VL by myself.

fakepaper56 planned changes to this revision.Jan 8 2023, 11:34 PM

I think I could support i64 vectors for the optimization. I will support it in the revision.

Support i64 vectors and rebase.

fakepaper56 retitled this revision from [RISCV] Teach lowerCTLZ_CTTZ_ZERO_UNDEF to handle i32 vectors with conversion to f32 vectors. to [RISCV] Teach lowerCTLZ_CTTZ_ZERO_UNDEF to handle conversioin i32/i64 vectors to f32 vectors..Jan 9 2023, 5:51 AM
fakepaper56 edited the summary of this revision. (Show Details)

Fix typos in commit message.

fakepaper56 retitled this revision from [RISCV] Teach lowerCTLZ_CTTZ_ZERO_UNDEF to handle conversioin i32/i64 vectors to f32 vectors. to [RISCV] Teach lowerCTLZ_CTTZ_ZERO_UNDEF to handle conversion i32/i64 vectors to f32 vectors..Jan 9 2023, 5:55 AM
craig.topper added inline comments.Jan 10 2023, 10:00 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
3555

I think we should use f64 for i64 if it is available. That avoids a narrowing conversion.

3557

FLoatVal -> FloatVal

3586

snrl ->vnsrl?

3588

ANY_EXTEND is allowed to place random values into the extra bits. I think you want ZERO_EXTEND here.

llvm/test/CodeGen/RISCV/rvv/cttz-sdnode.ll
1669

Missing CHECK-F and CHECK-D here?

Address Craig's comment.

craig.topper added inline comments.Jan 10 2023, 11:00 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
3586

This wasn't addressed

3588

This wasn't addressed

llvm/test/CodeGen/RISCV/rvv/ctlz-sdnode.ll
1082

Any idea where this vmset came from?

fakepaper56 marked 2 inline comments as done.

Address missing comment in last update.

fakepaper56 marked 4 inline comments as done.Jan 11 2023, 12:29 AM
fakepaper56 added inline comments.
llvm/test/CodeGen/RISCV/rvv/ctlz-sdnode.ll
1082

The PseudoVFCVT_RM_F_XU_V_<LMUL>_MASK does not have unmask version now. I think there are same issues about other RM conversion.

craig.topper accepted this revision.Jan 11 2023, 12:34 AM

LGTM

llvm/test/CodeGen/RISCV/rvv/ctlz-sdnode.ll
1082

ok thanks for checking.

This revision is now accepted and ready to land.Jan 11 2023, 12:34 AM

Rebase and update test cases.

This revision was landed with ongoing or failed builds.Jan 11 2023, 10:42 PM
This revision was automatically updated to reflect the committed changes.