This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add rvv codegen support for vp.fpext.
ClosedPublic

Authored by jacquesguan on Apr 18 2022, 7:42 PM.

Details

Summary

This patch adds rvv codegen support for vp.fpext. The lowering of fp_round, vp.fptrunc, fp_extend and vp.fpext share most code so use a common lowering function to handle these four.

And this patch changes the intermediate cast from ISD::FP_EXTEND / ISD::FP_ROUND to the RVV VL version op RISCVISD::FP_EXTEND_VL and RISCVISD::FP_ROUND_VL for scalable vectors.

Diff Detail

Event Timeline

jacquesguan created this revision.Apr 18 2022, 7:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2022, 7:42 PM
jacquesguan requested review of this revision.Apr 18 2022, 7:42 PM

If I've read this correctly, this isn't strictly the same behaviour for fixed-length ISD::FP_EXTEND because it used to use ISD::FP_EXTEND for intermediate extends, whereas now it consistently uses RISCVISD::FP_EXTEND_VL. I wonder if we silently went through the same change when ISD::FP_ROUND was merged with ISD::VP_FP_ROUND.

This isn't necessarily a problem as evidently the existing tests haven't changed, but it should probably be noted in the commit description at least.

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

You've removed the last use of getRVVFPExtendOrRound here, haven't you?

If I've read this correctly, this isn't strictly the same behaviour for fixed-length ISD::FP_EXTEND because it used to use ISD::FP_EXTEND for intermediate extends, whereas now it consistently uses RISCVISD::FP_EXTEND_VL. I wonder if we silently went through the same change when ISD::FP_ROUND was merged with ISD::VP_FP_ROUND.

This isn't necessarily a problem as evidently the existing tests haven't changed, but it should probably be noted in the commit description at least.

I think you mean the scalable vector? Yes, there is some logic change. Because ISD::FP_EXTEND and ISD::FP_ROUND has no mask and VL operands, if we want to keep the old way, maybe will import some complicated if-conditions. I am thinking should we remove those patterns in the RISCVInstrInfoVSDPatterns.td and let the function handle all lowering cases including fixed-length and scalable vector type? What's your opinion?

remove redundant function decleration.

jacquesguan added inline comments.Apr 25 2022, 12:41 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
3163

Done, I removed the decleration.

reames added a subscriber: reames.Apr 25 2022, 9:26 AM

ping.

Please update the description to mention the behavior change for scalable vectors as @frasercrmck requested.

jacquesguan edited the summary of this revision. (Show Details)May 6 2022, 7:28 PM

ping.

Please update the description to mention the behavior change for scalable vectors as @frasercrmck requested.

Done, I changed the revision's description.

frasercrmck accepted this revision.May 10 2022, 2:05 AM

LGTM, thanks!

This revision is now accepted and ready to land.May 10 2022, 2:05 AM
This revision was landed with ongoing or failed builds.May 10 2022, 8:28 PM
This revision was automatically updated to reflect the committed changes.