Page MenuHomePhabricator

[X86] Add custom type legalization and lowering for scalar STRICT_FP_TO_SINT/UINT
ClosedPublic

Authored by craig.topper on Nov 13 2019, 5:04 PM.

Details

Summary

This is a first pass at Custom lowering for these operations. I also updated some of the vector code where it was obviously easy and straightforward. More work needed in follow up.

This enables these operations to be handled with X87 where special rounding control adjustments are needed to perform a truncate.

Still need to fix Promotion in the target independent code in LegalizeDAG.
llrint/llround split into separate test file because we can't make a strict libcall properly yet either and we need to do that when i64 isn't a legal type.

This does not include any isel support. So we still rely on the mutation in SelectionDAGIsel to remove the strict from this stuff later. Except for the X87 stuff which goes through custom nodes that already had chains.

Diff Detail

Event Timeline

craig.topper created this revision.Nov 13 2019, 5:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 13 2019, 5:04 PM
Herald added a subscriber: hiraditya. · View Herald Transcript

I am working on STRICT_FP_TO_SINT/UINT, too. For now, Pengfei and I thought we can set promotion as custom so we can promote the type in x86 without affecting other backend. What do you think?

llvm/test/CodeGen/X86/fp-intrinsics.ll
1085

This is wrong for the strict case. If the converted value doesn't fit in the destination range, the invalid flag is supposed to be raised. Likewise with the AVX case below. Possibly also the x87 case above.

1087

Why is this retq?

1106

This comment seems wrong.

1111

I don't think this sequence or the SSE and AVX sequences are correct for strictfp.

Use strict fp fsub in the expansion of unsigned conversion to i64 on 32-bit targets without sse support for the floating point type. This technically can still raise a spurious inexact exception, but the same thing happens with the default expansion in LegalizeDAG we use on i64 targets.

Use a different form of getNode to avoid explicitly calling getVTList.

-Remove bad test comments.
-Enable cmov on the x87 test so that the unsigned conversion to i64 is a little simpler. There was a branch in it before, but it still didn't prevent us from speculating an fsub.

craig.topper marked 4 inline comments as done.Nov 15 2019, 9:59 AM
craig.topper added inline comments.
llvm/test/CodeGen/X86/fp-intrinsics.ll
1085

Agreed, but the only way I see to fix this is to add an IR pass to insert the checks before SelectionDAG.

1087

retq just indicates the size of the return address to pop. In 64-bit mode its always retq.

1111

Agreed, but I think fixing that is out of scope for this patch. I don't have any good ideas for how to fix it other than to introduce an IR pass to insert control flow before SelectionDAG.

Fix unused variable warning

llvm/test/CodeGen/X86/fp-intrinsics.ll
1085

Can you put FIXME's in the code indicating that something needs to be done. And maybe create a Bugzilla report?

Rebase after other changes that went in over the weekend.

This revision is now accepted and ready to land.Tue, Nov 19, 3:45 PM
This revision was automatically updated to reflect the committed changes.