[MLIR] Testing arith-to-emitc conversions using opaque types #137936
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-mlir Author: Niklas Degener (ndegener-amd) ChangesCurrently, most of the type conversions in ArithToEmitC do not support emitc opaque types. This is problematic if someone uses opaque types, as the testing has to be done locally. Full diff: https://github.com/llvm/llvm-project/pull/137936.diff 6 Files Affected:
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
index 57029c64ffd00..6adbb475cdbf8 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
@@ -46,6 +46,14 @@ bool isIntegerIndexOrOpaqueType(Type type);
/// Determines whether \p type is a valid floating-point type in EmitC.
bool isSupportedFloatType(mlir::Type type);
+/// Determines whether \p type is a valid floating-point or opaque type in
+/// EmitC.
+bool isFloatOrOpaqueType(mlir::Type type);
+
+/// Determines whether \p type is a valid integer or opaque type in
+/// EmitC.
+bool isIntegerOrOpaqueType(mlir::Type type);
+
/// Determines whether \p type is a emitc.size_t/ssize_t type.
bool isPointerWideType(mlir::Type type);
diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
index 359d7b2279639..88f8651618df0 100644
--- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
+++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
@@ -67,6 +67,7 @@ Type adaptIntegralTypeSignedness(Type ty, bool needsUnsigned) {
/// Insert a cast operation to type \p ty if \p val does not have this type.
Value adaptValueType(Value val, ConversionPatternRewriter &rewriter, Type ty) {
+ assert(emitc::isSupportedEmitCType(val.getType()));
return rewriter.createOrFold<emitc::CastOp>(val.getLoc(), ty, val);
}
@@ -273,7 +274,8 @@ class CmpIOpConversion : public OpConversionPattern<arith::CmpIOp> {
ConversionPatternRewriter &rewriter) const override {
Type type = adaptor.getLhs().getType();
- if (!type || !(isa<IntegerType>(type) || emitc::isPointerWideType(type))) {
+ if (!type || !(emitc::isIntegerOrOpaqueType(type) ||
+ emitc::isPointerWideType(type))) {
return rewriter.notifyMatchFailure(
op, "expected integer or size_t/ssize_t/ptrdiff_t type");
}
@@ -328,7 +330,7 @@ class CastConversion : public OpConversionPattern<ArithOp> {
ConversionPatternRewriter &rewriter) const override {
Type opReturnType = this->getTypeConverter()->convertType(op.getType());
- if (!opReturnType || !(isa<IntegerType>(opReturnType) ||
+ if (!opReturnType || !(emitc::isIntegerOrOpaqueType(opReturnType) ||
emitc::isPointerWideType(opReturnType)))
return rewriter.notifyMatchFailure(
op, "expected integer or size_t/ssize_t/ptrdiff_t result type");
@@ -339,7 +341,7 @@ class CastConversion : public OpConversionPattern<ArithOp> {
}
Type operandType = adaptor.getIn().getType();
- if (!operandType || !(isa<IntegerType>(operandType) ||
+ if (!operandType || !(emitc::isIntegerOrOpaqueType(operandType) ||
emitc::isPointerWideType(operandType)))
return rewriter.notifyMatchFailure(
op, "expected integer or size_t/ssize_t/ptrdiff_t operand type");
@@ -433,7 +435,8 @@ class BinaryUIOpConversion final : public OpConversionPattern<ArithOp> {
if (!newRetTy)
return rewriter.notifyMatchFailure(uiBinOp,
"converting result type failed");
- if (!isa<IntegerType>(newRetTy)) {
+
+ if (!emitc::isIntegerOrOpaqueType(newRetTy)) {
return rewriter.notifyMatchFailure(uiBinOp, "expected integer type");
}
Type unsignedType =
@@ -441,8 +444,8 @@ class BinaryUIOpConversion final : public OpConversionPattern<ArithOp> {
if (!unsignedType)
return rewriter.notifyMatchFailure(uiBinOp,
"converting result type failed");
- Value lhsAdapted = adaptValueType(uiBinOp.getLhs(), rewriter, unsignedType);
- Value rhsAdapted = adaptValueType(uiBinOp.getRhs(), rewriter, unsignedType);
+ Value lhsAdapted = adaptValueType(adaptor.getLhs(), rewriter, unsignedType);
+ Value rhsAdapted = adaptValueType(adaptor.getRhs(), rewriter, unsignedType);
auto newDivOp =
rewriter.create<EmitCOp>(uiBinOp.getLoc(), unsignedType,
@@ -463,7 +466,8 @@ class IntegerOpConversion final : public OpConversionPattern<ArithOp> {
ConversionPatternRewriter &rewriter) const override {
Type type = this->getTypeConverter()->convertType(op.getType());
- if (!type || !(isa<IntegerType>(type) || emitc::isPointerWideType(type))) {
+ if (!type || !(emitc::isIntegerOrOpaqueType(type) ||
+ emitc::isPointerWideType(type))) {
return rewriter.notifyMatchFailure(
op, "expected integer or size_t/ssize_t/ptrdiff_t type");
}
@@ -506,7 +510,7 @@ class BitwiseOpConversion : public OpConversionPattern<ArithOp> {
ConversionPatternRewriter &rewriter) const override {
Type type = this->getTypeConverter()->convertType(op.getType());
- if (!isa_and_nonnull<IntegerType>(type)) {
+ if (!type || !emitc::isIntegerOrOpaqueType(type)) {
return rewriter.notifyMatchFailure(
op,
"expected integer type, vector/tensor support not yet implemented");
@@ -546,7 +550,9 @@ class ShiftOpConversion : public OpConversionPattern<ArithOp> {
ConversionPatternRewriter &rewriter) const override {
Type type = this->getTypeConverter()->convertType(op.getType());
- if (!type || !(isa<IntegerType>(type) || emitc::isPointerWideType(type))) {
+ bool retIsOpaque = isa_and_nonnull<emitc::OpaqueType>(type);
+ if (!type || (!retIsOpaque && !(isa<IntegerType>(type) ||
+ emitc::isPointerWideType(type)))) {
return rewriter.notifyMatchFailure(
op, "expected integer or size_t/ssize_t/ptrdiff_t type");
}
@@ -572,21 +578,33 @@ class ShiftOpConversion : public OpConversionPattern<ArithOp> {
op.getLoc(), rhsType, "sizeof", ArrayRef<Value>{eight});
width = rewriter.create<emitc::MulOp>(op.getLoc(), rhsType, eight,
sizeOfCall.getResult(0));
- } else {
+ } else if (!retIsOpaque) {
width = rewriter.create<emitc::ConstantOp>(
op.getLoc(), rhsType,
rewriter.getIntegerAttr(rhsType, type.getIntOrFloatBitWidth()));
+ } else {
+ width = rewriter.create<emitc::ConstantOp>(
+ op.getLoc(), rhsType,
+ emitc::OpaqueAttr::get(rhsType.getContext(),
+ "opaque_shift_bitwidth"));
}
Value excessCheck = rewriter.create<emitc::CmpOp>(
op.getLoc(), rewriter.getI1Type(), emitc::CmpPredicate::lt, rhs, width);
// Any concrete value is a valid refinement of poison.
- Value poison = rewriter.create<emitc::ConstantOp>(
- op.getLoc(), arithmeticType,
- (isa<IntegerType>(arithmeticType)
- ? rewriter.getIntegerAttr(arithmeticType, 0)
- : rewriter.getIndexAttr(0)));
+ Value poison;
+ if (retIsOpaque) {
+ poison = rewriter.create<emitc::ConstantOp>(
+ op.getLoc(), arithmeticType,
+ emitc::OpaqueAttr::get(rhsType.getContext(), "opaque_shift_poison"));
+ } else {
+ poison = rewriter.create<emitc::ConstantOp>(
+ op.getLoc(), arithmeticType,
+ (isa<IntegerType>(arithmeticType)
+ ? rewriter.getIntegerAttr(arithmeticType, 0)
+ : rewriter.getIndexAttr(0)));
+ }
emitc::ExpressionOp ternary = rewriter.create<emitc::ExpressionOp>(
op.getLoc(), arithmeticType, /*do_not_inline=*/false);
@@ -663,19 +681,23 @@ class FtoICastOpConversion : public OpConversionPattern<CastOp> {
if (!dstType)
return rewriter.notifyMatchFailure(castOp, "type conversion failed");
+ Type actualResultType = dstType;
+
// Float-to-i1 casts are not supported: any value with 0 < value < 1 must be
// truncated to 0, whereas a boolean conversion would return true.
- if (!emitc::isSupportedIntegerType(dstType) || dstType.isInteger(1))
- return rewriter.notifyMatchFailure(castOp,
- "unsupported cast destination type");
-
- // Convert to unsigned if it's the "ui" variant
- // Signless is interpreted as signed, so no need to cast for "si"
- Type actualResultType = dstType;
- if (isa<arith::FPToUIOp>(castOp)) {
- actualResultType =
- rewriter.getIntegerType(dstType.getIntOrFloatBitWidth(),
- /*isSigned=*/false);
+ bool dstIsOpaque = isa<emitc::OpaqueType>(dstType);
+ if (!dstIsOpaque) {
+ if (!emitc::isSupportedIntegerType(dstType) || dstType.isInteger(1))
+ return rewriter.notifyMatchFailure(castOp,
+ "unsupported cast destination type");
+
+ // Convert to unsigned if it's the "ui" variant
+ // Signless is interpreted as signed, so no need to cast for "si"
+ if (isa<arith::FPToUIOp>(castOp)) {
+ actualResultType =
+ rewriter.getIntegerType(dstType.getIntOrFloatBitWidth(),
+ /*isSigned=*/false);
+ }
}
Value result = rewriter.create<emitc::CastOp>(
@@ -702,7 +724,9 @@ class ItoFCastOpConversion : public OpConversionPattern<CastOp> {
ConversionPatternRewriter &rewriter) const override {
// Vectors in particular are not supported
Type operandType = adaptor.getIn().getType();
- if (!emitc::isSupportedIntegerType(operandType))
+ bool opIsOpaque = isa<emitc::OpaqueType>(operandType);
+
+ if (!(opIsOpaque || emitc::isSupportedIntegerType(operandType)))
return rewriter.notifyMatchFailure(castOp,
"unsupported cast source type");
@@ -717,7 +741,7 @@ class ItoFCastOpConversion : public OpConversionPattern<CastOp> {
// Convert to unsigned if it's the "ui" variant
// Signless is interpreted as signed, so no need to cast for "si"
Type actualOperandType = operandType;
- if (isa<arith::UIToFPOp>(castOp)) {
+ if (!opIsOpaque && isa<arith::UIToFPOp>(castOp)) {
actualOperandType =
rewriter.getIntegerType(operandType.getIntOrFloatBitWidth(),
/*isSigned=*/false);
@@ -745,7 +769,7 @@ class FpCastOpConversion : public OpConversionPattern<CastOp> {
ConversionPatternRewriter &rewriter) const override {
// Vectors in particular are not supported.
Type operandType = adaptor.getIn().getType();
- if (!emitc::isSupportedFloatType(operandType))
+ if (!emitc::isFloatOrOpaqueType(operandType))
return rewriter.notifyMatchFailure(castOp,
"unsupported cast source type");
if (auto roundingModeOp =
@@ -759,7 +783,7 @@ class FpCastOpConversion : public OpConversionPattern<CastOp> {
if (!dstType)
return rewriter.notifyMatchFailure(castOp, "type conversion failed");
- if (!emitc::isSupportedFloatType(dstType))
+ if (!emitc::isFloatOrOpaqueType(dstType))
return rewriter.notifyMatchFailure(castOp,
"unsupported cast destination type");
diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitCPass.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitCPass.cpp
index 45a088ed144f1..30d5eed9dceaa 100644
--- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitCPass.cpp
+++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitCPass.cpp
@@ -30,9 +30,42 @@ namespace {
struct ConvertArithToEmitC
: public impl::ConvertArithToEmitCBase<ConvertArithToEmitC> {
void runOnOperation() override;
+
+ /// Applies conversion to opaque types for f80 and i80 types, both unsupported
+ /// in emitc. Used to test the pass with opaque types.
+ void populateOpaqueTypeConversions(TypeConverter &converter);
};
} // namespace
+void ConvertArithToEmitC::populateOpaqueTypeConversions(
+ TypeConverter &converter) {
+ converter.addConversion([](Type type) -> std::optional<Type> {
+ if (type.isF80())
+ return emitc::OpaqueType::get(type.getContext(), "f80");
+ if (type.isInteger() && type.getIntOrFloatBitWidth() == 80)
+ return emitc::OpaqueType::get(type.getContext(), "i80");
+ return type;
+ });
+
+ converter.addTypeAttributeConversion(
+ [](Type type,
+ Attribute attrToConvert) -> TypeConverter::AttributeConversionResult {
+ if (auto floatAttr = llvm::dyn_cast<FloatAttr>(attrToConvert)) {
+ if (floatAttr.getType().isF80()) {
+ return emitc::OpaqueAttr::get(type.getContext(), "f80");
+ }
+ return {};
+ }
+ if (auto intAttr = llvm::dyn_cast<IntegerAttr>(attrToConvert)) {
+ if (intAttr.getType().isInteger() &&
+ intAttr.getType().getIntOrFloatBitWidth() == 80) {
+ return emitc::OpaqueAttr::get(type.getContext(), "i80");
+ }
+ }
+ return {};
+ });
+}
+
void ConvertArithToEmitC::runOnOperation() {
ConversionTarget target(getContext());
@@ -42,8 +75,8 @@ void ConvertArithToEmitC::runOnOperation() {
RewritePatternSet patterns(&getContext());
TypeConverter typeConverter;
- typeConverter.addConversion([](Type type) { return type; });
+ populateOpaqueTypeConversions(typeConverter);
populateArithToEmitCPatterns(typeConverter, patterns);
if (failed(
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index b4d7482554fbc..c34752f92c794 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -132,6 +132,14 @@ bool mlir::emitc::isSupportedFloatType(Type type) {
return false;
}
+bool mlir::emitc::isIntegerOrOpaqueType(Type type) {
+ return isa<emitc::OpaqueType>(type) || isSupportedIntegerType(type);
+}
+
+bool mlir::emitc::isFloatOrOpaqueType(Type type) {
+ return isa<emitc::OpaqueType>(type) || isSupportedFloatType(type);
+}
+
bool mlir::emitc::isPointerWideType(Type type) {
return isa<emitc::SignedSizeTType, emitc::SizeTType, emitc::PtrDiffTType>(
type);
diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir
index 9850f336b5ad6..e652ed38a21d2 100644
--- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir
+++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir
@@ -14,13 +14,6 @@ func.func @arith_cast_vector(%arg0: vector<5xf32>) -> vector<5xi32> {
return %t: vector<5xi32>
}
-// -----
-func.func @arith_cast_f80(%arg0: f80) -> i32 {
- // expected-error @+1 {{failed to legalize operation 'arith.fptosi'}}
- %t = arith.fptosi %arg0 : f80 to i32
- return %t: i32
-}
-
// -----
func.func @arith_cast_f128(%arg0: f128) -> i32 {
@@ -29,15 +22,6 @@ func.func @arith_cast_f128(%arg0: f128) -> i32 {
return %t: i32
}
-
-// -----
-
-func.func @arith_cast_to_f80(%arg0: i32) -> f80 {
- // expected-error @+1 {{failed to legalize operation 'arith.sitofp'}}
- %t = arith.sitofp %arg0 : i32 to f80
- return %t: f80
-}
-
// -----
func.func @arith_cast_to_f128(%arg0: i32) -> f128 {
diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
index cb1d092918f03..3db27db653a80 100644
--- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
+++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
@@ -771,3 +771,74 @@ func.func @arith_truncf(%arg0: f64) -> f16 {
return %truncd1 : f16
}
+
+// -----
+
+func.func @float_opaque_conversion(%arg0: f80, %arg1: f80) {
+ // CHECK-LABEL: float_opaque_conversion
+ // CHECK-SAME: (%[[Arg0:[^ ]*]]: f80, %[[Arg1:[^ ]*]]: f80)
+
+ // CHECK-DAG: [[arg1_cast:[^ ]*]] = builtin.unrealized_conversion_cast %[[Arg1]] : f80 to !emitc.opaque<"f80">
+ // CHECK-DAG: [[arg0_cast:[^ ]*]] = builtin.unrealized_conversion_cast %[[Arg0]] : f80 to !emitc.opaque<"f80">
+ // CHECK: "emitc.constant"() <{value = #emitc.opaque<"f80">}> : () -> !emitc.opaque<"f80">
+ %10 = arith.constant 0.0 : f80
+ // CHECK: emitc.add [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"f80">, !emitc.opaque<"f80">) -> !emitc.opaque<"f80">
+ %2 = arith.addf %arg0, %arg1 : f80
+ // CHECK: [[EQ:[^ ]*]] = emitc.cmp eq, [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"f80">, !emitc.opaque<"f80">) -> i1
+ // CHECK: [[NotNaNArg0:[^ ]*]] = emitc.cmp eq, [[arg0_cast]], [[arg0_cast]] : (!emitc.opaque<"f80">, !emitc.opaque<"f80">) -> i1
+ // CHECK: [[NotNaNArg1:[^ ]*]] = emitc.cmp eq, [[arg1_cast]], [[arg1_cast]] : (!emitc.opaque<"f80">, !emitc.opaque<"f80">) -> i1
+ // CHECK: [[Ordered:[^ ]*]] = emitc.logical_and [[NotNaNArg0]], [[NotNaNArg1]] : i1, i1
+ // CHECK: emitc.logical_and [[Ordered]], [[EQ]] : i1, i1
+ %11 = arith.cmpf oeq, %arg0, %arg1 : f80
+ // CHECK: emitc.unary_minus [[arg0_cast]] : (!emitc.opaque<"f80">) -> !emitc.opaque<"f80">
+ %12 = arith.negf %arg0 : f80
+ // CHECK: [[V0:[^ ]*]] = emitc.cast [[arg0_cast]] : !emitc.opaque<"f80"> to ui32
+ // CHECK: [[V1:[^ ]*]] = emitc.cast [[V0]] : ui32 to i32
+ %7 = arith.fptoui %arg0 : f80 to i32
+ // CHECK: emitc.cast [[V1]] : i32 to !emitc.opaque<"f80">
+ %8 = arith.sitofp %7 : i32 to f80
+ // CHECK: [[trunc:[^ ]*]] = emitc.cast [[arg0_cast]] : !emitc.opaque<"f80"> to f32
+ %13 = arith.truncf %arg0 : f80 to f32
+ // CHECK: emitc.cast [[trunc]] : f32 to !emitc.opaque<"f80">
+ %15 = arith.extf %13 : f32 to f80
+ return
+}
+
+// -----
+
+func.func @int_opaque_conversion(%arg0: i80, %arg1: i80, %arg2: i1) {
+ // CHECK-LABEL: int_opaque_conversion
+ // CHECK-SAME: (%[[Arg0:[^ ]*]]: i80, %[[Arg1:[^ ]*]]: i80, %[[Arg2:[^ ]*]]: i1)
+
+ // CHECK-DAG: [[arg1_cast:[^ ]*]] = builtin.unrealized_conversion_cast %[[Arg1]] : i80 to !emitc.opaque<"i80">
+ // CHECK-DAG: [[arg0_cast:[^ ]*]] = builtin.unrealized_conversion_cast %[[Arg0]] : i80 to !emitc.opaque<"i80">
+ // CHECK: "emitc.constant"() <{value = #emitc.opaque<"i80">}> : () -> !emitc.opaque<"i80">
+ %10 = arith.constant 0 : i80
+ // CHECK: emitc.div [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> !emitc.opaque<"i80">
+ %3 = arith.divui %arg0, %arg1 : i80
+ // CHECK: emitc.add [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> !emitc.opaque<"i80">
+ %2 = arith.addi %arg0, %arg1 : i80
+ // CHECK: emitc.bitwise_and [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> !emitc.opaque<"i80">
+ %14 = arith.andi %arg0, %arg1 : i80
+ // CHECK: [[Bitwidth:[^ ]*]] = "emitc.constant"() <{value = #emitc.opaque<"opaque_shift_bitwidth">}> : () -> !emitc.opaque<"i80">
+ // CHECK: [[LT:[^ ]*]] = emitc.cmp lt, [[arg1_cast]], [[Bitwidth]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> i1
+ // CHECK: [[Poison:[^ ]*]] = "emitc.constant"() <{value = #emitc.opaque<"opaque_shift_poison">}> : () -> !emitc.opaque<"i80">
+ // CHECK: [[Exp:[^ ]*]] = emitc.expression : !emitc.opaque<"i80"> {
+ // CHECK: [[LShift:[^ ]*]] = emitc.bitwise_left_shift [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> !emitc.opaque<"i80">
+ // CHECK: emitc.conditional [[LT]], [[LShift]], [[Poison]] : !emitc.opaque<"i80">
+ // CHECK: emitc.yield {{.*}} : !emitc.opaque<"i80">
+ // CHECK: }
+ %12 = arith.shli %arg0, %arg1 : i80
+ // CHECK: emitc.cmp eq, [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> i1
+ %11 = arith.cmpi eq, %arg0, %arg1 : i80
+ // CHECK: emitc.conditional %[[Arg2]], [[arg0_cast]], [[arg1_cast]] : !emitc.opaque<"i80">
+ %13 = arith.select %arg2, %arg0, %arg1 : i80
+ // CHECK: [[V0:[^ ]*]] = emitc.cast [[arg0_cast]] : !emitc.opaque<"i80"> to ui8
+ // CHECK: emitc.cast [[V0]] : ui8 to i8
+ %15 = arith.trunci %arg0 : i80 to i8
+ // CHECK: [[V1:[^ ]*]] = emitc.cast [[arg0_cast]] : !emitc.opaque<"i80"> to f32
+ %9 = arith.uitofp %arg0 : i80 to f32
+ // CHECK: emitc.cast [[V1]] : f32 to !emitc.opaque<"i80">
+ %6 = arith.fptosi %9 : f32 to i80
+ return
+}
|
|
@llvm/pr-subscribers-mlir-emitc Author: Niklas Degener (ndegener-amd) ChangesCurrently, most of the type conversions in ArithToEmitC do not support emitc opaque types. This is problematic if someone uses opaque types, as the testing has to be done locally. Full diff: https://github.com/llvm/llvm-project/pull/137936.diff 6 Files Affected:
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
index 57029c64ffd00..6adbb475cdbf8 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
@@ -46,6 +46,14 @@ bool isIntegerIndexOrOpaqueType(Type type);
/// Determines whether \p type is a valid floating-point type in EmitC.
bool isSupportedFloatType(mlir::Type type);
+/// Determines whether \p type is a valid floating-point or opaque type in
+/// EmitC.
+bool isFloatOrOpaqueType(mlir::Type type);
+
+/// Determines whether \p type is a valid integer or opaque type in
+/// EmitC.
+bool isIntegerOrOpaqueType(mlir::Type type);
+
/// Determines whether \p type is a emitc.size_t/ssize_t type.
bool isPointerWideType(mlir::Type type);
diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
index 359d7b2279639..88f8651618df0 100644
--- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
+++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
@@ -67,6 +67,7 @@ Type adaptIntegralTypeSignedness(Type ty, bool needsUnsigned) {
/// Insert a cast operation to type \p ty if \p val does not have this type.
Value adaptValueType(Value val, ConversionPatternRewriter &rewriter, Type ty) {
+ assert(emitc::isSupportedEmitCType(val.getType()));
return rewriter.createOrFold<emitc::CastOp>(val.getLoc(), ty, val);
}
@@ -273,7 +274,8 @@ class CmpIOpConversion : public OpConversionPattern<arith::CmpIOp> {
ConversionPatternRewriter &rewriter) const override {
Type type = adaptor.getLhs().getType();
- if (!type || !(isa<IntegerType>(type) || emitc::isPointerWideType(type))) {
+ if (!type || !(emitc::isIntegerOrOpaqueType(type) ||
+ emitc::isPointerWideType(type))) {
return rewriter.notifyMatchFailure(
op, "expected integer or size_t/ssize_t/ptrdiff_t type");
}
@@ -328,7 +330,7 @@ class CastConversion : public OpConversionPattern<ArithOp> {
ConversionPatternRewriter &rewriter) const override {
Type opReturnType = this->getTypeConverter()->convertType(op.getType());
- if (!opReturnType || !(isa<IntegerType>(opReturnType) ||
+ if (!opReturnType || !(emitc::isIntegerOrOpaqueType(opReturnType) ||
emitc::isPointerWideType(opReturnType)))
return rewriter.notifyMatchFailure(
op, "expected integer or size_t/ssize_t/ptrdiff_t result type");
@@ -339,7 +341,7 @@ class CastConversion : public OpConversionPattern<ArithOp> {
}
Type operandType = adaptor.getIn().getType();
- if (!operandType || !(isa<IntegerType>(operandType) ||
+ if (!operandType || !(emitc::isIntegerOrOpaqueType(operandType) ||
emitc::isPointerWideType(operandType)))
return rewriter.notifyMatchFailure(
op, "expected integer or size_t/ssize_t/ptrdiff_t operand type");
@@ -433,7 +435,8 @@ class BinaryUIOpConversion final : public OpConversionPattern<ArithOp> {
if (!newRetTy)
return rewriter.notifyMatchFailure(uiBinOp,
"converting result type failed");
- if (!isa<IntegerType>(newRetTy)) {
+
+ if (!emitc::isIntegerOrOpaqueType(newRetTy)) {
return rewriter.notifyMatchFailure(uiBinOp, "expected integer type");
}
Type unsignedType =
@@ -441,8 +444,8 @@ class BinaryUIOpConversion final : public OpConversionPattern<ArithOp> {
if (!unsignedType)
return rewriter.notifyMatchFailure(uiBinOp,
"converting result type failed");
- Value lhsAdapted = adaptValueType(uiBinOp.getLhs(), rewriter, unsignedType);
- Value rhsAdapted = adaptValueType(uiBinOp.getRhs(), rewriter, unsignedType);
+ Value lhsAdapted = adaptValueType(adaptor.getLhs(), rewriter, unsignedType);
+ Value rhsAdapted = adaptValueType(adaptor.getRhs(), rewriter, unsignedType);
auto newDivOp =
rewriter.create<EmitCOp>(uiBinOp.getLoc(), unsignedType,
@@ -463,7 +466,8 @@ class IntegerOpConversion final : public OpConversionPattern<ArithOp> {
ConversionPatternRewriter &rewriter) const override {
Type type = this->getTypeConverter()->convertType(op.getType());
- if (!type || !(isa<IntegerType>(type) || emitc::isPointerWideType(type))) {
+ if (!type || !(emitc::isIntegerOrOpaqueType(type) ||
+ emitc::isPointerWideType(type))) {
return rewriter.notifyMatchFailure(
op, "expected integer or size_t/ssize_t/ptrdiff_t type");
}
@@ -506,7 +510,7 @@ class BitwiseOpConversion : public OpConversionPattern<ArithOp> {
ConversionPatternRewriter &rewriter) const override {
Type type = this->getTypeConverter()->convertType(op.getType());
- if (!isa_and_nonnull<IntegerType>(type)) {
+ if (!type || !emitc::isIntegerOrOpaqueType(type)) {
return rewriter.notifyMatchFailure(
op,
"expected integer type, vector/tensor support not yet implemented");
@@ -546,7 +550,9 @@ class ShiftOpConversion : public OpConversionPattern<ArithOp> {
ConversionPatternRewriter &rewriter) const override {
Type type = this->getTypeConverter()->convertType(op.getType());
- if (!type || !(isa<IntegerType>(type) || emitc::isPointerWideType(type))) {
+ bool retIsOpaque = isa_and_nonnull<emitc::OpaqueType>(type);
+ if (!type || (!retIsOpaque && !(isa<IntegerType>(type) ||
+ emitc::isPointerWideType(type)))) {
return rewriter.notifyMatchFailure(
op, "expected integer or size_t/ssize_t/ptrdiff_t type");
}
@@ -572,21 +578,33 @@ class ShiftOpConversion : public OpConversionPattern<ArithOp> {
op.getLoc(), rhsType, "sizeof", ArrayRef<Value>{eight});
width = rewriter.create<emitc::MulOp>(op.getLoc(), rhsType, eight,
sizeOfCall.getResult(0));
- } else {
+ } else if (!retIsOpaque) {
width = rewriter.create<emitc::ConstantOp>(
op.getLoc(), rhsType,
rewriter.getIntegerAttr(rhsType, type.getIntOrFloatBitWidth()));
+ } else {
+ width = rewriter.create<emitc::ConstantOp>(
+ op.getLoc(), rhsType,
+ emitc::OpaqueAttr::get(rhsType.getContext(),
+ "opaque_shift_bitwidth"));
}
Value excessCheck = rewriter.create<emitc::CmpOp>(
op.getLoc(), rewriter.getI1Type(), emitc::CmpPredicate::lt, rhs, width);
// Any concrete value is a valid refinement of poison.
- Value poison = rewriter.create<emitc::ConstantOp>(
- op.getLoc(), arithmeticType,
- (isa<IntegerType>(arithmeticType)
- ? rewriter.getIntegerAttr(arithmeticType, 0)
- : rewriter.getIndexAttr(0)));
+ Value poison;
+ if (retIsOpaque) {
+ poison = rewriter.create<emitc::ConstantOp>(
+ op.getLoc(), arithmeticType,
+ emitc::OpaqueAttr::get(rhsType.getContext(), "opaque_shift_poison"));
+ } else {
+ poison = rewriter.create<emitc::ConstantOp>(
+ op.getLoc(), arithmeticType,
+ (isa<IntegerType>(arithmeticType)
+ ? rewriter.getIntegerAttr(arithmeticType, 0)
+ : rewriter.getIndexAttr(0)));
+ }
emitc::ExpressionOp ternary = rewriter.create<emitc::ExpressionOp>(
op.getLoc(), arithmeticType, /*do_not_inline=*/false);
@@ -663,19 +681,23 @@ class FtoICastOpConversion : public OpConversionPattern<CastOp> {
if (!dstType)
return rewriter.notifyMatchFailure(castOp, "type conversion failed");
+ Type actualResultType = dstType;
+
// Float-to-i1 casts are not supported: any value with 0 < value < 1 must be
// truncated to 0, whereas a boolean conversion would return true.
- if (!emitc::isSupportedIntegerType(dstType) || dstType.isInteger(1))
- return rewriter.notifyMatchFailure(castOp,
- "unsupported cast destination type");
-
- // Convert to unsigned if it's the "ui" variant
- // Signless is interpreted as signed, so no need to cast for "si"
- Type actualResultType = dstType;
- if (isa<arith::FPToUIOp>(castOp)) {
- actualResultType =
- rewriter.getIntegerType(dstType.getIntOrFloatBitWidth(),
- /*isSigned=*/false);
+ bool dstIsOpaque = isa<emitc::OpaqueType>(dstType);
+ if (!dstIsOpaque) {
+ if (!emitc::isSupportedIntegerType(dstType) || dstType.isInteger(1))
+ return rewriter.notifyMatchFailure(castOp,
+ "unsupported cast destination type");
+
+ // Convert to unsigned if it's the "ui" variant
+ // Signless is interpreted as signed, so no need to cast for "si"
+ if (isa<arith::FPToUIOp>(castOp)) {
+ actualResultType =
+ rewriter.getIntegerType(dstType.getIntOrFloatBitWidth(),
+ /*isSigned=*/false);
+ }
}
Value result = rewriter.create<emitc::CastOp>(
@@ -702,7 +724,9 @@ class ItoFCastOpConversion : public OpConversionPattern<CastOp> {
ConversionPatternRewriter &rewriter) const override {
// Vectors in particular are not supported
Type operandType = adaptor.getIn().getType();
- if (!emitc::isSupportedIntegerType(operandType))
+ bool opIsOpaque = isa<emitc::OpaqueType>(operandType);
+
+ if (!(opIsOpaque || emitc::isSupportedIntegerType(operandType)))
return rewriter.notifyMatchFailure(castOp,
"unsupported cast source type");
@@ -717,7 +741,7 @@ class ItoFCastOpConversion : public OpConversionPattern<CastOp> {
// Convert to unsigned if it's the "ui" variant
// Signless is interpreted as signed, so no need to cast for "si"
Type actualOperandType = operandType;
- if (isa<arith::UIToFPOp>(castOp)) {
+ if (!opIsOpaque && isa<arith::UIToFPOp>(castOp)) {
actualOperandType =
rewriter.getIntegerType(operandType.getIntOrFloatBitWidth(),
/*isSigned=*/false);
@@ -745,7 +769,7 @@ class FpCastOpConversion : public OpConversionPattern<CastOp> {
ConversionPatternRewriter &rewriter) const override {
// Vectors in particular are not supported.
Type operandType = adaptor.getIn().getType();
- if (!emitc::isSupportedFloatType(operandType))
+ if (!emitc::isFloatOrOpaqueType(operandType))
return rewriter.notifyMatchFailure(castOp,
"unsupported cast source type");
if (auto roundingModeOp =
@@ -759,7 +783,7 @@ class FpCastOpConversion : public OpConversionPattern<CastOp> {
if (!dstType)
return rewriter.notifyMatchFailure(castOp, "type conversion failed");
- if (!emitc::isSupportedFloatType(dstType))
+ if (!emitc::isFloatOrOpaqueType(dstType))
return rewriter.notifyMatchFailure(castOp,
"unsupported cast destination type");
diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitCPass.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitCPass.cpp
index 45a088ed144f1..30d5eed9dceaa 100644
--- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitCPass.cpp
+++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitCPass.cpp
@@ -30,9 +30,42 @@ namespace {
struct ConvertArithToEmitC
: public impl::ConvertArithToEmitCBase<ConvertArithToEmitC> {
void runOnOperation() override;
+
+ /// Applies conversion to opaque types for f80 and i80 types, both unsupported
+ /// in emitc. Used to test the pass with opaque types.
+ void populateOpaqueTypeConversions(TypeConverter &converter);
};
} // namespace
+void ConvertArithToEmitC::populateOpaqueTypeConversions(
+ TypeConverter &converter) {
+ converter.addConversion([](Type type) -> std::optional<Type> {
+ if (type.isF80())
+ return emitc::OpaqueType::get(type.getContext(), "f80");
+ if (type.isInteger() && type.getIntOrFloatBitWidth() == 80)
+ return emitc::OpaqueType::get(type.getContext(), "i80");
+ return type;
+ });
+
+ converter.addTypeAttributeConversion(
+ [](Type type,
+ Attribute attrToConvert) -> TypeConverter::AttributeConversionResult {
+ if (auto floatAttr = llvm::dyn_cast<FloatAttr>(attrToConvert)) {
+ if (floatAttr.getType().isF80()) {
+ return emitc::OpaqueAttr::get(type.getContext(), "f80");
+ }
+ return {};
+ }
+ if (auto intAttr = llvm::dyn_cast<IntegerAttr>(attrToConvert)) {
+ if (intAttr.getType().isInteger() &&
+ intAttr.getType().getIntOrFloatBitWidth() == 80) {
+ return emitc::OpaqueAttr::get(type.getContext(), "i80");
+ }
+ }
+ return {};
+ });
+}
+
void ConvertArithToEmitC::runOnOperation() {
ConversionTarget target(getContext());
@@ -42,8 +75,8 @@ void ConvertArithToEmitC::runOnOperation() {
RewritePatternSet patterns(&getContext());
TypeConverter typeConverter;
- typeConverter.addConversion([](Type type) { return type; });
+ populateOpaqueTypeConversions(typeConverter);
populateArithToEmitCPatterns(typeConverter, patterns);
if (failed(
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index b4d7482554fbc..c34752f92c794 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -132,6 +132,14 @@ bool mlir::emitc::isSupportedFloatType(Type type) {
return false;
}
+bool mlir::emitc::isIntegerOrOpaqueType(Type type) {
+ return isa<emitc::OpaqueType>(type) || isSupportedIntegerType(type);
+}
+
+bool mlir::emitc::isFloatOrOpaqueType(Type type) {
+ return isa<emitc::OpaqueType>(type) || isSupportedFloatType(type);
+}
+
bool mlir::emitc::isPointerWideType(Type type) {
return isa<emitc::SignedSizeTType, emitc::SizeTType, emitc::PtrDiffTType>(
type);
diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir
index 9850f336b5ad6..e652ed38a21d2 100644
--- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir
+++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir
@@ -14,13 +14,6 @@ func.func @arith_cast_vector(%arg0: vector<5xf32>) -> vector<5xi32> {
return %t: vector<5xi32>
}
-// -----
-func.func @arith_cast_f80(%arg0: f80) -> i32 {
- // expected-error @+1 {{failed to legalize operation 'arith.fptosi'}}
- %t = arith.fptosi %arg0 : f80 to i32
- return %t: i32
-}
-
// -----
func.func @arith_cast_f128(%arg0: f128) -> i32 {
@@ -29,15 +22,6 @@ func.func @arith_cast_f128(%arg0: f128) -> i32 {
return %t: i32
}
-
-// -----
-
-func.func @arith_cast_to_f80(%arg0: i32) -> f80 {
- // expected-error @+1 {{failed to legalize operation 'arith.sitofp'}}
- %t = arith.sitofp %arg0 : i32 to f80
- return %t: f80
-}
-
// -----
func.func @arith_cast_to_f128(%arg0: i32) -> f128 {
diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
index cb1d092918f03..3db27db653a80 100644
--- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
+++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir
@@ -771,3 +771,74 @@ func.func @arith_truncf(%arg0: f64) -> f16 {
return %truncd1 : f16
}
+
+// -----
+
+func.func @float_opaque_conversion(%arg0: f80, %arg1: f80) {
+ // CHECK-LABEL: float_opaque_conversion
+ // CHECK-SAME: (%[[Arg0:[^ ]*]]: f80, %[[Arg1:[^ ]*]]: f80)
+
+ // CHECK-DAG: [[arg1_cast:[^ ]*]] = builtin.unrealized_conversion_cast %[[Arg1]] : f80 to !emitc.opaque<"f80">
+ // CHECK-DAG: [[arg0_cast:[^ ]*]] = builtin.unrealized_conversion_cast %[[Arg0]] : f80 to !emitc.opaque<"f80">
+ // CHECK: "emitc.constant"() <{value = #emitc.opaque<"f80">}> : () -> !emitc.opaque<"f80">
+ %10 = arith.constant 0.0 : f80
+ // CHECK: emitc.add [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"f80">, !emitc.opaque<"f80">) -> !emitc.opaque<"f80">
+ %2 = arith.addf %arg0, %arg1 : f80
+ // CHECK: [[EQ:[^ ]*]] = emitc.cmp eq, [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"f80">, !emitc.opaque<"f80">) -> i1
+ // CHECK: [[NotNaNArg0:[^ ]*]] = emitc.cmp eq, [[arg0_cast]], [[arg0_cast]] : (!emitc.opaque<"f80">, !emitc.opaque<"f80">) -> i1
+ // CHECK: [[NotNaNArg1:[^ ]*]] = emitc.cmp eq, [[arg1_cast]], [[arg1_cast]] : (!emitc.opaque<"f80">, !emitc.opaque<"f80">) -> i1
+ // CHECK: [[Ordered:[^ ]*]] = emitc.logical_and [[NotNaNArg0]], [[NotNaNArg1]] : i1, i1
+ // CHECK: emitc.logical_and [[Ordered]], [[EQ]] : i1, i1
+ %11 = arith.cmpf oeq, %arg0, %arg1 : f80
+ // CHECK: emitc.unary_minus [[arg0_cast]] : (!emitc.opaque<"f80">) -> !emitc.opaque<"f80">
+ %12 = arith.negf %arg0 : f80
+ // CHECK: [[V0:[^ ]*]] = emitc.cast [[arg0_cast]] : !emitc.opaque<"f80"> to ui32
+ // CHECK: [[V1:[^ ]*]] = emitc.cast [[V0]] : ui32 to i32
+ %7 = arith.fptoui %arg0 : f80 to i32
+ // CHECK: emitc.cast [[V1]] : i32 to !emitc.opaque<"f80">
+ %8 = arith.sitofp %7 : i32 to f80
+ // CHECK: [[trunc:[^ ]*]] = emitc.cast [[arg0_cast]] : !emitc.opaque<"f80"> to f32
+ %13 = arith.truncf %arg0 : f80 to f32
+ // CHECK: emitc.cast [[trunc]] : f32 to !emitc.opaque<"f80">
+ %15 = arith.extf %13 : f32 to f80
+ return
+}
+
+// -----
+
+func.func @int_opaque_conversion(%arg0: i80, %arg1: i80, %arg2: i1) {
+ // CHECK-LABEL: int_opaque_conversion
+ // CHECK-SAME: (%[[Arg0:[^ ]*]]: i80, %[[Arg1:[^ ]*]]: i80, %[[Arg2:[^ ]*]]: i1)
+
+ // CHECK-DAG: [[arg1_cast:[^ ]*]] = builtin.unrealized_conversion_cast %[[Arg1]] : i80 to !emitc.opaque<"i80">
+ // CHECK-DAG: [[arg0_cast:[^ ]*]] = builtin.unrealized_conversion_cast %[[Arg0]] : i80 to !emitc.opaque<"i80">
+ // CHECK: "emitc.constant"() <{value = #emitc.opaque<"i80">}> : () -> !emitc.opaque<"i80">
+ %10 = arith.constant 0 : i80
+ // CHECK: emitc.div [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> !emitc.opaque<"i80">
+ %3 = arith.divui %arg0, %arg1 : i80
+ // CHECK: emitc.add [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> !emitc.opaque<"i80">
+ %2 = arith.addi %arg0, %arg1 : i80
+ // CHECK: emitc.bitwise_and [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> !emitc.opaque<"i80">
+ %14 = arith.andi %arg0, %arg1 : i80
+ // CHECK: [[Bitwidth:[^ ]*]] = "emitc.constant"() <{value = #emitc.opaque<"opaque_shift_bitwidth">}> : () -> !emitc.opaque<"i80">
+ // CHECK: [[LT:[^ ]*]] = emitc.cmp lt, [[arg1_cast]], [[Bitwidth]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> i1
+ // CHECK: [[Poison:[^ ]*]] = "emitc.constant"() <{value = #emitc.opaque<"opaque_shift_poison">}> : () -> !emitc.opaque<"i80">
+ // CHECK: [[Exp:[^ ]*]] = emitc.expression : !emitc.opaque<"i80"> {
+ // CHECK: [[LShift:[^ ]*]] = emitc.bitwise_left_shift [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> !emitc.opaque<"i80">
+ // CHECK: emitc.conditional [[LT]], [[LShift]], [[Poison]] : !emitc.opaque<"i80">
+ // CHECK: emitc.yield {{.*}} : !emitc.opaque<"i80">
+ // CHECK: }
+ %12 = arith.shli %arg0, %arg1 : i80
+ // CHECK: emitc.cmp eq, [[arg0_cast]], [[arg1_cast]] : (!emitc.opaque<"i80">, !emitc.opaque<"i80">) -> i1
+ %11 = arith.cmpi eq, %arg0, %arg1 : i80
+ // CHECK: emitc.conditional %[[Arg2]], [[arg0_cast]], [[arg1_cast]] : !emitc.opaque<"i80">
+ %13 = arith.select %arg2, %arg0, %arg1 : i80
+ // CHECK: [[V0:[^ ]*]] = emitc.cast [[arg0_cast]] : !emitc.opaque<"i80"> to ui8
+ // CHECK: emitc.cast [[V0]] : ui8 to i8
+ %15 = arith.trunci %arg0 : i80 to i8
+ // CHECK: [[V1:[^ ]*]] = emitc.cast [[arg0_cast]] : !emitc.opaque<"i80"> to f32
+ %9 = arith.uitofp %arg0 : i80 to f32
+ // CHECK: emitc.cast [[V1]] : f32 to !emitc.opaque<"i80">
+ %6 = arith.fptosi %9 : f32 to i80
+ return
+}
|
|
Requesting review from @simon-camp @marbre @aniragil . |
marbre
reviewed
Apr 30, 2025
|
Yes, sorry. I will take a look at what went wrong |
|
The fork wasn't up-to-date and I somehow managed to run the wrong tests. The issue was caused by the fact, that some conversions that previously did work for opaque types, were in the meantime changed in a way that made them fail as well. I now included those conversions in the changes. |
simon-camp
reviewed
May 2, 2025
| @@ -572,21 +588,33 @@ class ShiftOpConversion : public OpConversionPattern<ArithOp> { | |||
| op.getLoc(), rhsType, "sizeof", ArrayRef<Value>{eight}); | |||
| width = rewriter.create<emitc::MulOp>(op.getLoc(), rhsType, eight, | |||
| sizeOfCall.getResult(0)); | |||
| } else { | |||
| } else if (!retIsOpaque) { | |||
There was a problem hiding this comment.
You could invert the condition and swap the branches.
| width = rewriter.create<emitc::ConstantOp>( | ||
| op.getLoc(), rhsType, | ||
| emitc::OpaqueAttr::get(rhsType.getContext(), | ||
| "opaque_shift_bitwidth")); |
There was a problem hiding this comment.
Where does opaque_shift_bitwidth come from?
There was a problem hiding this comment.
If opaque types are used, the bitwidth, which is needed for the shiftOp, can't be determined. So the opaque attribute serves as a reference point for where to enter the bitwidth of the type later on.
| if (retIsOpaque) { | ||
| poison = rewriter.create<emitc::ConstantOp>( | ||
| op.getLoc(), arithmeticType, | ||
| emitc::OpaqueAttr::get(rhsType.getContext(), "opaque_shift_poison")); |
There was a problem hiding this comment.
Same as above, where is this defined and why is it needed?
| }; | ||
| } // namespace | ||
|
|
||
| void ConvertArithToEmitC::populateOpaqueTypeConversions( |
There was a problem hiding this comment.
I don't understand why these types should be unconditionally legalized, and why only for bitwidth 80?
|
I'm gonna elaborate a bit on our use case. We had the issue, that our own opaque type conversions couldn't be tested in llvm and we had to do it locally. But for testing opaque type conversions, first of all a conversion is needed which can be tested. So the idea was to use some types, that are unsupported in emitc anyway, and convert them to opaque types. There is no specific reason for choosing f80 and i80 except them not being supported. As you mentioned, this does introduce the issue that i80 and f80 are unconditionally legalized and no longer recognized as unsupported. But the ArithToEmitC pass is essentially only a testing pass, so I think it shouldn't matter. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, most of the type conversions in ArithToEmitC do not support emitc opaque types. This is problematic if someone uses opaque types, as the testing has to be done locally.
To fix this, opaque types are legalized for the conversions. Additionally, i80 and f80 types, both unsupported in emitc, are converted into opaque types in the arith-to-emitc pass. This way, the handling of opaque types can be properly tested in the added test cases for float and integer types.