Page MenuHomePhabricator

[X86] Do not try to custom-lower sitofp/fptosi in soft-float mode
ClosedPublic

Authored by mkuper on Nov 9 2015, 3:53 AM.

Diff Detail

Event Timeline

mkuper updated this revision to Diff 39678.Nov 9 2015, 3:53 AM
mkuper retitled this revision from to [X86] Do not try to custom-lower sitofp/fptosi in soft-float mode.
mkuper updated this object.
mkuper added reviewers: nadav, mbodart.
mkuper added a subscriber: llvm-commits.
nadav edited edge metadata.Nov 9 2015, 7:01 AM

LGTM.

mbodart edited edge metadata.Nov 9 2015, 10:45 AM

The changes look good, but I have a couple questions on the requirements of soft float support.

Is soft float expected to work for f80? If so, can you please add some test cases?

The test only covers 32-bit targets. Is it expected to work for x86_64?

Minor comment on the test function names.
While it's unlikely to be problematic in the context of this test,
note that "l" and "ul" are substrings of "ll" and "ull", and could
inadvertently match where unintended. Names like u32 and s64
would fix that.

mkuper added a comment.Nov 9 2015, 1:37 PM

Thanks, Mitch.

I'll change the test names.

Regarding your questions - right now, my main concern is f32 and f64 on 32-bit. But it's also expected to work on x86_64, I'll add the appropriate test.
I really don't know about f80 (or f128) support for soft-float on x86, but there's nothing special there for FP_TO_SINT. (There's really nothing special for f32 and f64 either, aside from the fact it was broken in a way other floating point ops weren't...)

This revision was automatically updated to reflect the committed changes.