This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Fold fp_to_int(ftrunc (X)) -> fp_to_int(X) for vector.
Needs ReviewPublic

Authored by jacquesguan on Aug 22 2022, 11:17 PM.

Details

Summary

This revision adds vector support for performFP_TO_INTCombine. Because we only have static rtz for rvv convert instruction, so only fold ftrunc. I leave performFP_TO_INT_SATCombine not modified, if this way was reviewed as a right one, I would change performFP_TO_INT_SATCombine then.

Diff Detail

Event Timeline

jacquesguan created this revision.Aug 22 2022, 11:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 22 2022, 11:17 PM
jacquesguan requested review of this revision.Aug 22 2022, 11:17 PM
craig.topper added inline comments.Aug 26 2022, 5:54 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
8862

I'm not sure we should do this. There is a slight semantic difference. If this was legal to replace (fp_to_int (ftrunc V)) with fp_to_int (V) we'd just do that in generic DAGCombine or InstCombine for all targets.

I need to think through exactly what bad thing can happen.

jacquesguan added inline comments.Sep 5 2022, 11:57 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
8862

According to llvm langref:
'The fptoui instruction converts its floating-point operand into the nearest (rounding towards zero) unsigned integer value.'
And the llvm.trunc.* intrinsics is same with trunc of libm, which is descripted as : 'If no errors occur, the nearest integer value not greater in magnitude than arg (in other words, arg rounded towards zero), is returned.'

So I think they both are strict rounding towards zero.

I tried common DAGCombiner at first, it would affect some other targets' test, so I switch to RISCV target specificed DAG combiner.

craig.topper added inline comments.Feb 15 2023, 1:03 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
8830

A different vector implementation was implemented in D141599