[mlir][emitc] Support convert arith.extf and arith.truncf to emitc

[jacquesguan]

No description provided.

[llvmbot]

@llvm/pr-subscribers-mlir-emitc

@llvm/pr-subscribers-mlir

Author: Jianjian Guan (jacquesguan)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/121184.diff

2 Files Affected:

• (modified) mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp (+34-1)
• (modified) mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir (+26)

diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
index ccbc1669b7a92a..e2fbac40517e0d 100644
--- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
+++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
@@ -733,6 +733,37 @@ class ItoFCastOpConversion : public OpConversionPattern<CastOp> {
   }
 };
 
+// Floating-point to floating-point conversions.
+template <typename CastOp>
+class FpCastOpConversion : public OpConversionPattern<CastOp> {
+public:
+  FpCastOpConversion(const TypeConverter &typeConverter, MLIRContext *context)
+      : OpConversionPattern<CastOp>(typeConverter, context) {}
+
+  LogicalResult
+  matchAndRewrite(CastOp castOp, typename CastOp::Adaptor adaptor,
+                  ConversionPatternRewriter &rewriter) const override {
+    // Vectors in particular are not supported
+    Type operandType = adaptor.getIn().getType();
+    if (!emitc::isSupportedFloatType(operandType))
+      return rewriter.notifyMatchFailure(castOp,
+                                         "unsupported cast source type");
+
+    Type dstType = this->getTypeConverter()->convertType(castOp.getType());
+    if (!dstType)
+      return rewriter.notifyMatchFailure(castOp, "type conversion failed");
+
+    if (!emitc::isSupportedFloatType(dstType))
+      return rewriter.notifyMatchFailure(castOp,
+                                         "unsupported cast destination type");
+
+    Value fpCastOperand = adaptor.getIn();
+    rewriter.replaceOpWithNewOp<emitc::CastOp>(castOp, dstType, fpCastOperand);
+
+    return success();
+  }
+};
+
 } // namespace
 
 //===----------------------------------------------------------------------===//
@@ -778,7 +809,9 @@ void mlir::populateArithToEmitCPatterns(TypeConverter &typeConverter,
     ItoFCastOpConversion<arith::SIToFPOp>,
     ItoFCastOpConversion<arith::UIToFPOp>,
     FtoICastOpConversion<arith::FPToSIOp>,
-    FtoICastOpConversion<arith::FPToUIOp>
+    FtoICastOpConversion<arith::FPToUIOp>,
+    FpCastOpConversion<arith::ExtFOp>,
+    FpCastOpConversion<arith::TruncFOp>
   >(typeConverter, ctx);
   // clang-format on
 }
diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
index 1728c3a2557e07..434f8771d58c1e 100644
--- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
+++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
@@ -739,3 +739,29 @@ func.func @arith_divui_remui(%arg0: i32, %arg1: i32) -> i32 {
 
   return %div : i32
 }
+
+// -----
+
+func.func @arith_extf(%arg0: f16) -> f64 {
+  // CHECK-LABEL: arith_extf
+  // CHECK-SAME: (%[[Arg0:[^ ]*]]: f16)
+  // CHECK: %[[Extd0:.*]] = emitc.cast %[[Arg0]] : f16 to f32
+  %extd0 = arith.extf %arg0 : f16 to f32
+  // CHECK: %[[Extd1:.*]] = emitc.cast %[[Extd0]] : f32 to f64
+  %extd1 = arith.extf %extd0 : f32 to f64
+
+  return %extd1 : f64
+}
+
+// -----
+
+func.func @arith_truncf(%arg0: f64) -> f16 {
+  // CHECK-LABEL: arith_truncf
+  // CHECK-SAME: (%[[Arg0:[^ ]*]]: f64)
+  // CHECK: %[[Truncd0:.*]] = emitc.cast %[[Arg0]] : f64 to f32
+  %truncd0 = arith.truncf %arg0 : f64 to f32
+  // CHECK: %[[Truncd1:.*]] = emitc.cast %[[Truncd0]] : f32 to f16
+  %truncd1 = arith.truncf %truncd0 : f32 to f16
+
+  return %truncd1 : f16
+}
\ No newline at end of file

[CoTinker]

Looks good to me, and adds a new line at end of the file maybe better.😊

[marbre]

Thanks for adding further conversion! Adding a newline to the test would be indeed preferred. i am ooo and need to take a closer look when i am back but I wonder what the expectations with regards to casting and rounding are. arith.trunc states

If the value cannot be exactly represented, it is rounded using the provided rounding mode or the default one if no rounding mode is provided.

but this seems not to be handled in the conversion.

[jacquesguan]

Looks good to me, and adds a new line at end of the file maybe better.😊

Thanks, I added a end line.

[jacquesguan]

Thanks for adding further conversion! Adding a newline to the test would be indeed preferred. i am ooo and need to take a closer look when i am back but I wonder what the expectations with regards to casting and rounding are. arith.trunc states

If the value cannot be exactly represented, it is rounded using the provided rounding mode or the default one if no rounding mode is provided.

but this seems not to be handled in the conversion.

Thanks for comment. I added a early exit when the cast op has rounding mode.

[TinaAMD]

Thanks for the PR!

[TinaAMD]

LGTM!

[marbre]

Without looking further into the underlying code, is a RoundingModeAttr also provided for the default rounding mode? I think a test as suggested by @TinaAMD would help.

Furthermore, do we know what the default rounding mode is that's assumed by Arith? And is this what we would expect with C or C++? For C, https://en.cppreference.com/w/c/language/conversion (Real floating-point conversions) says

A value of any real floating type can be implicitly converted to any other real floating type.

• If the value can be represented by the target type exactly, it is unchanged
• if the value can be represented, but cannot be represented exactly, the result is the nearest higher or the nearest lower value (in other words, rounding direction is implementation-defined), although if IEEE arithmetic is supported, rounding is to nearest
• if the value cannot be represented, the behavior is undefined
  | This section is incompleteReason: check IEEE if appropriately-signed infinity is required |

[jacquesguan]

As ArithToSPIRV and ArithToAMDGPU also don't support the conversion of truncf with RoundingModeAttr now, I think we could leave it to future implementation?

[marbre]

Fair enough. The one difference however might be that the SPRI-V as well as the amdgpu dialect specify what rounding modes they assume in general. Maybe we could add at least some documentation to e.g. emitc.td?

[jacquesguan]

Can you point the position where these 2 dialects specify the default rounding mode? I don't find that.

[marbre]

Well at least some ops specify who the rounding is done, see for example https://mlir.llvm.org/docs/Dialects/SPIR-V/#spirvclfma-spirvclfmaop. But I haven't had time to look into ArithToSPRIV to see what is the mapping between ops and what is done about rounding in the conversion. EmitC does not need to be as specific in the op descriptions but maybe it's work to document what can be expected by the conversion somewhere in a (new?) section in emitc.td. WDYT @simon-camp @mgehre-amd?

[simon-camp]

I assume the arith dialect implicitly follows the LLVM LangRef, which would mean the default of rounding ties to even is used. arith::TruncF is also lowered to llvm::LLVM::FPTruncOp when missing the rounding mode attr. So this should match with the C behaviour IIUC. But adding a section to the docs about assumptions would be a good idea.

[jacquesguan]

OK, so I think maybe create a new PR to clarify the rounding mode in EmitC dialect and leave this PR only affect the conversion of arith.extf and arith.truncf?

[marbre]

Yeah, I think this should be fine :)

[marbre]

NIT:

Suggested change

	// Vectors in particular are not supported
	// Vectors in particular are not supported.