This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add support for vp.fptosi where the result is a mask type.
ClosedPublic

Authored by craig.topper on Mar 29 2022, 11:57 AM.

Details

Summary

We can do this conversion by converting the same sized integer type, then compare the result with 0. The conversion is undefined if the converted FP value doesn't fit in an i1.

Diff Detail

Event Timeline

craig.topper created this revision.Mar 29 2022, 11:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2022, 11:57 AM
craig.topper requested review of this revision.Mar 29 2022, 11:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2022, 11:57 AM

I must be missing something very obvious: wouldn't 2.0 round to 2 and then give us i1 0 because its LSB is 0?

I must be missing something very obvious: wouldn't 2.0 round to 2 and then give us i1 0 because its LSB is 0?

2.0 is out of range for the integer so it is UB and produces poison.

I must be missing something very obvious: wouldn't 2.0 round to 2 and then give us i1 0 because its LSB is 0?

2.0 is out of range for the integer so it is UB and produces poison.

Ah right! Thanks. I'd say to remove the extra masking, checking other targets don't seem to bother.

Just curious why you chose a narrowing conversion rather than a regular one, perhaps you want to reduce the register pressure for the result or the expectation is that it'll be faster?

I must be missing something very obvious: wouldn't 2.0 round to 2 and then give us i1 0 because its LSB is 0?

2.0 is out of range for the integer so it is UB and produces poison.

Ah right! Thanks. I'd say to remove the extra masking, checking other targets don't seem to bother.

Just curious why you chose a narrowing conversion rather than a regular one, perhaps you want to reduce the register pressure for the result or the expectation is that it'll be faster?

I copied from the regular FP_TO_SINT code. Which is also where I got the AND since it went through TRUNCATE lowering.

craig.topper updated this revision to Diff 419905.EditedApr 1 2022, 6:47 PM

Change to a single width conversion and remove the AND.
Add unsigned support

craig.topper edited the summary of this revision. (Show Details)Apr 1 2022, 6:50 PM
frasercrmck accepted this revision.Apr 5 2022, 8:42 AM

LGTM, thanks. I suspect the patterns will clash with those in D122743 but they should be functionally equivalent.

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

comment's out of date: no more and?

This revision is now accepted and ready to land.Apr 5 2022, 8:42 AM
This revision was landed with ongoing or failed builds.Apr 5 2022, 9:48 AM
This revision was automatically updated to reflect the committed changes.