Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jacquesguan merged 4 commits into from
Jan 16, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce6db11
[mlir][emitc] Support convert arith.extf and arith.truncf to emitc
jacquesguan Dec 27, 2024
ddb67d2
Early exit with rounding mode
jacquesguan Dec 31, 2024
5e184f0
Add more unsupport cases
jacquesguan Jan 2, 2025
3913e5f
fix test
jacquesguan Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,43 @@ 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");
if (auto roundingModeOp =
dyn_cast<arith::ArithRoundingModeInterface>(*castOp)) {
// Only supporting default rounding mode as of now.
if (roundingModeOp.getRoundingModeAttr())
Copy link
Member

@marbre marbre Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 |

Copy link
Contributor Author

@jacquesguan jacquesguan Jan 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@marbre marbre Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@jacquesguan jacquesguan Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@marbre marbre Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

@simon-camp simon-camp Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@jacquesguan jacquesguan Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@marbre marbre Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think this should be fine :)

return rewriter.notifyMatchFailure(castOp, "unsupported rounding mode");
}

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

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -778,7 +815,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
}
40 changes: 40 additions & 0 deletions mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,43 @@ func.func @arith_remui_vector(%arg0: vector<5xi32>, %arg1: vector<5xi32>) -> vec
%divui = arith.remui %arg0, %arg1 : vector<5xi32>
return %divui: vector<5xi32>
}

// -----

func.func @arith_truncf(%arg0: f64) -> f32 {
// expected-error @+1 {{failed to legalize operation 'arith.truncf'}}
%truncd = arith.truncf %arg0 to_nearest_away : f64 to f32
return %truncd : f32
}

// -----

func.func @arith_extf_f128(%arg0: f32) -> f128 {
// expected-error @+1 {{failed to legalize operation 'arith.extf'}}
%extd = arith.extf %arg0 : f32 to f128
return %extd : f128
}

// -----

func.func @arith_truncf_f128(%arg0: f128) -> f32 {
// expected-error @+1 {{failed to legalize operation 'arith.truncf'}}
%truncd = arith.truncf %arg0 : f128 to f32
return %truncd : f32
}

// -----

func.func @arith_extf_vector(%arg0: vector<4xf32>) -> vector<4xf64> {
// expected-error @+1 {{failed to legalize operation 'arith.extf'}}
%extd = arith.extf %arg0 : vector<4xf32> to vector<4xf64>
return %extd : vector<4xf64>
}

// -----

func.func @arith_truncf_vector(%arg0: vector<4xf64>) -> vector<4xf32> {
// expected-error @+1 {{failed to legalize operation 'arith.truncf'}}
%truncd = arith.truncf %arg0 : vector<4xf64> to vector<4xf32>
return %truncd : vector<4xf32>
}
26 changes: 26 additions & 0 deletions mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading