[flang-commits] [flang] [flang] Use saturated intrinsic for floating point conversions (PR #130686)

[via flang-commits]

================
@@ -835,10 +835,20 @@ struct ConvertOpConversion : public fir::FIROpConversion<fir::ConvertOp> {
         return mlir::success();
       }
       if (mlir::isa<mlir::IntegerType>(toTy)) {
-        if (toTy.isUnsignedInteger())
-          rewriter.replaceOpWithNewOp<mlir::LLVM::FPToUIOp>(convert, toTy, op0);
-        else
-          rewriter.replaceOpWithNewOp<mlir::LLVM::FPToSIOp>(convert, toTy, op0);
+        // NOTE: We are checking the fir type here because toTy is an LLVM type
+        // which is signless, and we need to use the intrinsic that matches the
+        // sign of the output in fir.
+        if (toFirTy.isUnsignedInteger()) {
+          auto intrinsicName =
+              mlir::StringAttr::get(convert.getContext(), "llvm.fptoui.sat");
+          rewriter.replaceOpWithNewOp<mlir::LLVM::CallIntrinsicOp>(
+              convert, toTy, intrinsicName, op0);
+        } else {
+          auto intrinsicName =
+              mlir::StringAttr::get(convert.getContext(), "llvm.fptosi.sat");
+          rewriter.replaceOpWithNewOp<mlir::LLVM::CallIntrinsicOp>(
+              convert, toTy, intrinsicName, op0);
+        }
----------------
jeanPerier wrote:

> Do you think using the instructions instead of the saturated intrinsics under (for example) -ffast-math would be a good compromise if performance issues show up?

Personally, I agree with that approach. I think it is better to avoid having too many code generation paths unless there is an actual use case for it, in which case ` -ffast-math` would sounds like the right flag to deviate from the requirements.

Please wait for @kiranchandramohan's feedback on the matter.

https://github.com/llvm/llvm-project/pull/130686