Page MenuHomePhabricator

Implement FPToUI and UIToFP ops in standard dialect
ClosedPublic

Authored by mars1026 on Aug 7 2020, 2:20 PM.

Details

Summary

Add the unsigned complements to the existing FPToSI and SIToFP operations in the
standard dialect, with one-to-one lowerings to the corresponding LLVM operations.

Diff Detail

Event Timeline

mars1026 created this revision.Aug 7 2020, 2:20 PM
Herald added a project: Restricted Project. · View Herald Transcript
mars1026 requested review of this revision.Aug 7 2020, 2:20 PM
flaub added a subscriber: flaub.Aug 8 2020, 12:43 AM
ftynse requested changes to this revision.Aug 10 2020, 2:18 AM

Please post an RFC at https://llvm.discourse.group for any additions to the standard dialect.

mlir/lib/Dialect/StandardOps/IR/Ops.cpp
1787

MLIR supports unsigned integers natively, why is this restricted to signless integers only?

This revision now requires changes to proceed.Aug 10 2020, 2:18 AM
mars1026 added inline comments.Aug 10 2020, 12:07 PM
mlir/lib/Dialect/StandardOps/IR/Ops.cpp
1787

Consistency with the existing FPToSIOp, which only applies to conversions between floats and signless integers. It would make sense to me that FPToSIOp should also accept signed integer types, and FPToUIOp should accept unsigned integers, but that would expand the scope of this proposal.

rriddle added inline comments.Aug 10 2020, 12:11 PM
mlir/lib/Dialect/StandardOps/IR/Ops.cpp
1787

The standard dialect does not operate on signed/unsigned types, it is explicitly signless.

https://mlir.llvm.org/docs/Rationale/Rationale/#integer-signedness-semantics

flaub added a comment.Aug 18 2020, 8:57 PM

Friendly poke, wondering what the status of this is. Looks like we have an RFC here https://llvm.discourse.group/t/rfc-standard-add-std-fptoui-and-std-uitofp-operations/1567. AFAICT, there aren't any outstanding issues. This is a required op for certain situations. Because the standard dialect only deals with 'signless' integers, we must be able to reintroduce signedness in a few key areas, one of them being casts, otherwise we get incorrect behavior (we've run into this in real-world situations).

ftynse accepted this revision.Aug 19 2020, 4:04 AM
This revision is now accepted and ready to land.Aug 19 2020, 4:04 AM
This revision was landed with ongoing or failed builds.Aug 19 2020, 1:49 PM
This revision was automatically updated to reflect the committed changes.