From 6da0548b478748dc3db41fe3149529b4087a31a6 Mon Sep 17 00:00:00 2001 From: Matthias Gehre Date: Thu, 4 Apr 2024 07:29:37 +0200 Subject: [PATCH 1/9] ArithToEmitC: lower cmpi, trunci & index_cast --- .../Conversion/ArithToEmitC/ArithToEmitC.cpp | 24 ++++++++++++- .../ArithToEmitC/arith-to-emitc.mlir | 36 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp index 0337314ce7f34..52a8ffae25c2b 100644 --- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp +++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp @@ -244,7 +244,6 @@ class CmpIOpConversion : public OpConversionPattern { } llvm_unreachable("unknown cmpi predicate kind"); } - LogicalResult matchAndRewrite(arith::CmpIOp op, OpAdaptor adaptor, ConversionPatternRewriter &rewriter) const override { @@ -274,6 +273,27 @@ class CmpIOpConversion : public OpConversionPattern { } }; +template +class CastConversion : public OpConversionPattern { +public: + using OpConversionPattern::OpConversionPattern; + + LogicalResult + matchAndRewrite(ArithOp op, typename ArithOp::Adaptor adaptor, + ConversionPatternRewriter &rewriter) const override { + + Type type = this->getTypeConverter()->convertType(op.getType()); + if (!isa_and_nonnull(type)) { + return rewriter.notifyMatchFailure(op, "expected integer type"); + } + + auto result = rewriter.template create(op.getLoc(), type, + adaptor.getIn()); + rewriter.replaceOp(op, result); + return success(); + } +}; + template class ArithOpConversion final : public OpConversionPattern { public: @@ -478,6 +498,8 @@ void mlir::populateArithToEmitCPatterns(TypeConverter &typeConverter, CmpFOpConversion, CmpIOpConversion, SelectOpConversion, + CastConversion, + CastConversion, ItoFCastOpConversion, ItoFCastOpConversion, FtoICastOpConversion, diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir index ed63d40808973..c3720a4839863 100644 --- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir +++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir @@ -390,3 +390,39 @@ func.func @arith_int_to_float_cast_ops(%arg0: i8, %arg1: i64) { return } + +// ----- + +func.func @arith_cmpi_eq(%arg0: i32, %arg1: i32) -> i1 { + // CHECK-LABEL: arith_cmpi_eq + // CHECK-SAME: ([[Arg0:[^ ]*]]: i32, [[Arg1:[^ ]*]]: i32) + // CHECK-DAG: [[EQ:[^ ]*]] = emitc.cmp eq, [[Arg0]], [[Arg1]] : (i32, i32) -> i1 + %eq = arith.cmpi eq, %arg0, %arg1 : i32 + // CHECK: return [[EQ]] + return %eq: i1 +} + +func.func @arith_cmpi_ult(%arg0: i32, %arg1: i32) -> i1 { + // CHECK-LABEL: arith_cmpi_ult + // CHECK-SAME: ([[Arg0:[^ ]*]]: i32, [[Arg1:[^ ]*]]: i32) + // CHECK-DAG: [[CastArg0:[^ ]*]] = emitc.cast [[Arg0]] : i32 to ui32 + // CHECK-DAG: [[CastArg1:[^ ]*]] = emitc.cast [[Arg1]] : i32 to ui32 + // CHECK-DAG: [[ULT:[^ ]*]] = emitc.cmp lt, [[CastArg0]], [[CastArg1]] : (ui32, ui32) -> i1 + %eq = arith.cmpi ult, %arg0, %arg1 : i32 + // CHECK: return [[ULT]] + return %eq: i1 +} + +// ----- + +func.func @casts(%arg0: i32) { + // CHECK-LABEL: casts + // CHECK-SAME: ([[Arg0:[^ ]*]]: i32) + // CHECK: emitc.cast [[Arg0]] : i32 to i8 + arith.trunci %arg0 : i32 to i8 + + %idx = arith.index_cast %arg0 : i32 to index + arith.index_cast %idx : index to i32 + + return +} From 3da633848c633cab136de9f540b0b6dff004bc0d Mon Sep 17 00:00:00 2001 From: Corentin Ferry Date: Mon, 6 May 2024 13:16:51 +0100 Subject: [PATCH 2/9] Reinstate blank line --- mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp index 52a8ffae25c2b..5ee5058c22ad5 100644 --- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp +++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp @@ -244,6 +244,7 @@ class CmpIOpConversion : public OpConversionPattern { } llvm_unreachable("unknown cmpi predicate kind"); } + LogicalResult matchAndRewrite(arith::CmpIOp op, OpAdaptor adaptor, ConversionPatternRewriter &rewriter) const override { From f1f43a06372586938b48233ca39d238da86c643c Mon Sep 17 00:00:00 2001 From: Corentin Ferry Date: Mon, 6 May 2024 14:07:07 +0100 Subject: [PATCH 3/9] Incorporate extsi --- .../Conversion/ArithToEmitC/ArithToEmitC.cpp | 1 + .../ArithToEmitC/arith-to-emitc.mlir | 33 ++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp index 5ee5058c22ad5..600e452f5608d 100644 --- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp +++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp @@ -500,6 +500,7 @@ void mlir::populateArithToEmitCPatterns(TypeConverter &typeConverter, CmpIOpConversion, SelectOpConversion, CastConversion, + CastConversion, CastConversion, ItoFCastOpConversion, ItoFCastOpConversion, diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir index c3720a4839863..5fd74b9802e5e 100644 --- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir +++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir @@ -415,14 +415,37 @@ func.func @arith_cmpi_ult(%arg0: i32, %arg1: i32) -> i1 { // ----- -func.func @casts(%arg0: i32) { - // CHECK-LABEL: casts +func.func @trunci(%arg0: i32) -> i8 { + // CHECK-LABEL: trunci // CHECK-SAME: ([[Arg0:[^ ]*]]: i32) // CHECK: emitc.cast [[Arg0]] : i32 to i8 - arith.trunci %arg0 : i32 to i8 + %truncd = arith.trunci %arg0 : i32 to i8 + + return %truncd : i8 +} + +// ----- + +func.func @index_cast(%arg0: i32) -> i32 { + // CHECK-LABEL: index_cast + // CHECK-SAME: ([[Arg0:[^ ]*]]: i32) + // CHECK: %[[ICast:.*]] = emitc.cast [[Arg0]] : i32 to index + // CHECK: emitc.cast %[[ICast]] : index to i32 %idx = arith.index_cast %arg0 : i32 to index - arith.index_cast %idx : index to i32 + %int = arith.index_cast %idx : index to i32 - return + return %int : i32 } + +// ----- + +func.func @extsi(%arg0: i32) { + // CHECK-LABEL: extsi + // CHECK-SAME: ([[Arg0:[^ ]*]]: i32) + // CHECK: emitc.cast [[Arg0]] : i32 to i64 + + %extd = arith.extsi %arg0 : i32 to i64 + + return +} \ No newline at end of file From 5c3d7bc00af2d06664dcac8e8fd8a8ec27be59be Mon Sep 17 00:00:00 2001 From: Corentin Ferry Date: Mon, 6 May 2024 15:08:41 +0100 Subject: [PATCH 4/9] Add support for extui, index_castui --- .../Conversion/ArithToEmitC/ArithToEmitC.cpp | 59 ++++++++++++++++--- .../ArithToEmitC/arith-to-emitc.mlir | 30 ++++++++++ 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp index 600e452f5608d..ec2def5bcb298 100644 --- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp +++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp @@ -274,7 +274,7 @@ class CmpIOpConversion : public OpConversionPattern { } }; -template +template class CastConversion : public OpConversionPattern { public: using OpConversionPattern::OpConversionPattern; @@ -283,13 +283,52 @@ class CastConversion : public OpConversionPattern { matchAndRewrite(ArithOp op, typename ArithOp::Adaptor adaptor, ConversionPatternRewriter &rewriter) const override { - Type type = this->getTypeConverter()->convertType(op.getType()); - if (!isa_and_nonnull(type)) { - return rewriter.notifyMatchFailure(op, "expected integer type"); + Type opReturnType = this->getTypeConverter()->convertType(op.getType()); + if (!isa_and_nonnull(opReturnType)) { + return rewriter.notifyMatchFailure(op, "expected result integer type"); + } + + if (adaptor.getOperands().size() > 1) { + return rewriter.notifyMatchFailure( + op, "CastConversion only supports unary ops"); + } + + Type operandType = + this->getTypeConverter()->convertType(adaptor.getIn().getType()); + if (!isa_and_nonnull(operandType)) { + return rewriter.notifyMatchFailure(op, "expected operand integer type"); + } + + Type castType = opReturnType; + // For int conversions: if the op is a ui variant and the type wanted as + // return type isn't unsigned, we need to issue an unsigned type to do + // the conversion. + if (isa(opReturnType) && + castType.isUnsignedInteger() != needsUnsigned) + castType = rewriter.getIntegerType(opReturnType.getIntOrFloatBitWidth(), + /*isSigned=*/!needsUnsigned); + + Value actualOp = adaptor.getIn(); + // Fix the signedness of the operand if necessary + if (isa(operandType) && + operandType.isUnsignedInteger() != needsUnsigned) { + Type correctSignednessType = + rewriter.getIntegerType(operandType.getIntOrFloatBitWidth(), + /*isSigned=*/!needsUnsigned); + actualOp = rewriter.template create( + op.getLoc(), correctSignednessType, actualOp); + } + + auto result = rewriter.template create(op.getLoc(), castType, + actualOp); + + // Fix the signedness of what this operation returns (for integers, + // the arith ops want signless results) + if (castType != opReturnType) { + result = rewriter.template create(op.getLoc(), + opReturnType, result); } - auto result = rewriter.template create(op.getLoc(), type, - adaptor.getIn()); rewriter.replaceOp(op, result); return success(); } @@ -499,9 +538,11 @@ void mlir::populateArithToEmitCPatterns(TypeConverter &typeConverter, CmpFOpConversion, CmpIOpConversion, SelectOpConversion, - CastConversion, - CastConversion, - CastConversion, + CastConversion, + CastConversion, + CastConversion, + CastConversion, + CastConversion, ItoFCastOpConversion, ItoFCastOpConversion, FtoICastOpConversion, diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir index 5fd74b9802e5e..fc29a98c13775 100644 --- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir +++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir @@ -440,6 +440,22 @@ func.func @index_cast(%arg0: i32) -> i32 { // ----- +func.func @index_castui(%arg0: i32) -> i32 { + // CHECK-LABEL: index_castui + // CHECK-SAME: (%[[Arg0:[^ ]*]]: i32) + // CHECK: %[[IToUI:.*]] = emitc.cast %[[Arg0]] : i32 to ui32 + // CHECK: %[[IdxCast:.*]] = emitc.cast %[[IToUI]] : ui32 to index + // CHECK: %[[UIntCast:.*]] = emitc.cast %[[IdxCast]] : index to ui32 + // CHECK: emitc.cast %[[UIntCast]] : ui32 to i32 + + %idx = arith.index_castui %arg0 : i32 to index + %int = arith.index_castui %idx : index to i32 + + return %int : i32 +} + +// ----- + func.func @extsi(%arg0: i32) { // CHECK-LABEL: extsi // CHECK-SAME: ([[Arg0:[^ ]*]]: i32) @@ -447,5 +463,19 @@ func.func @extsi(%arg0: i32) { %extd = arith.extsi %arg0 : i32 to i64 + return +} + +// ----- + +func.func @extui(%arg0: i32) { + // CHECK-LABEL: extui + // CHECK-SAME: (%[[Arg0:[^ ]*]]: i32) + // CHECK: %[[Conv0:.*]] = emitc.cast %[[Arg0]] : i32 to ui32 + // CHECK: %[[Conv1:.*]] = emitc.cast %[[Conv0]] : ui32 to ui64 + // CHECK: emitc.cast %[[Conv1]] : ui64 to i64 + + %extd = arith.extui %arg0 : i32 to i64 + return } \ No newline at end of file From 194e6699444ddb580c62c876ae69cf2453ba448d Mon Sep 17 00:00:00 2001 From: Corentin Ferry Date: Mon, 6 May 2024 15:29:02 +0100 Subject: [PATCH 5/9] TruncI works using unsigned --- mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp | 3 ++- mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp index ec2def5bcb298..a64d79bfa6b3e 100644 --- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp +++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp @@ -538,7 +538,8 @@ void mlir::populateArithToEmitCPatterns(TypeConverter &typeConverter, CmpFOpConversion, CmpIOpConversion, SelectOpConversion, - CastConversion, + // Truncation is guaranteed for unsigned types. + CastConversion, CastConversion, CastConversion, CastConversion, diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir index fc29a98c13775..40a15df1b19fd 100644 --- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir +++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir @@ -417,8 +417,10 @@ func.func @arith_cmpi_ult(%arg0: i32, %arg1: i32) -> i1 { func.func @trunci(%arg0: i32) -> i8 { // CHECK-LABEL: trunci - // CHECK-SAME: ([[Arg0:[^ ]*]]: i32) - // CHECK: emitc.cast [[Arg0]] : i32 to i8 + // CHECK-SAME: (%[[Arg0:[^ ]*]]: i32) + // CHECK: %[[CastUI:.*]] = emitc.cast %[[Arg0]] : i32 to ui32 + // CHECK: %[[Trunc:.*]] = emitc.cast %[[CastUI]] : ui32 to ui8 + // CHECK: emitc.cast %[[Trunc]] : ui8 to i8 %truncd = arith.trunci %arg0 : i32 to i8 return %truncd : i8 From 2458f9bf986368de7a2ff61cc8ba7b5b95cd7035 Mon Sep 17 00:00:00 2001 From: Corentin Ferry Date: Mon, 6 May 2024 16:05:22 +0100 Subject: [PATCH 6/9] Use unsigned whenever we know we're truncating --- .../Conversion/ArithToEmitC/ArithToEmitC.cpp | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp index a64d79bfa6b3e..6e6fc591ca9ba 100644 --- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp +++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp @@ -299,22 +299,28 @@ class CastConversion : public OpConversionPattern { return rewriter.notifyMatchFailure(op, "expected operand integer type"); } + bool doUnsigned = + needsUnsigned || + (isa(operandType) && isa(opReturnType) && + operandType.getIntOrFloatBitWidth() > + opReturnType.getIntOrFloatBitWidth()); + Type castType = opReturnType; // For int conversions: if the op is a ui variant and the type wanted as // return type isn't unsigned, we need to issue an unsigned type to do // the conversion. if (isa(opReturnType) && - castType.isUnsignedInteger() != needsUnsigned) + castType.isUnsignedInteger() != doUnsigned) castType = rewriter.getIntegerType(opReturnType.getIntOrFloatBitWidth(), - /*isSigned=*/!needsUnsigned); + /*isSigned=*/!doUnsigned); Value actualOp = adaptor.getIn(); // Fix the signedness of the operand if necessary if (isa(operandType) && - operandType.isUnsignedInteger() != needsUnsigned) { + operandType.isUnsignedInteger() != doUnsigned) { Type correctSignednessType = rewriter.getIntegerType(operandType.getIntOrFloatBitWidth(), - /*isSigned=*/!needsUnsigned); + /*isSigned=*/!doUnsigned); actualOp = rewriter.template create( op.getLoc(), correctSignednessType, actualOp); } @@ -334,6 +340,16 @@ class CastConversion : public OpConversionPattern { } }; +template +class UnsignedCastConversion : public CastConversion { + using CastConversion::CastConversion; +}; + +template +class SignedCastConversion : public CastConversion { + using CastConversion::CastConversion; +}; + template class ArithOpConversion final : public OpConversionPattern { public: @@ -539,11 +555,11 @@ void mlir::populateArithToEmitCPatterns(TypeConverter &typeConverter, CmpIOpConversion, SelectOpConversion, // Truncation is guaranteed for unsigned types. - CastConversion, - CastConversion, - CastConversion, - CastConversion, - CastConversion, + UnsignedCastConversion, + SignedCastConversion, + UnsignedCastConversion, + SignedCastConversion, + UnsignedCastConversion, ItoFCastOpConversion, ItoFCastOpConversion, FtoICastOpConversion, From 383a273baadd3a802438899a1847796259f26085 Mon Sep 17 00:00:00 2001 From: Corentin Ferry Date: Tue, 7 May 2024 07:08:39 +0100 Subject: [PATCH 7/9] Remove cmpi tests --- .../ArithToEmitC/arith-to-emitc.mlir | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir index 40a15df1b19fd..0a4d16b132e1d 100644 --- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir +++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir @@ -393,28 +393,6 @@ func.func @arith_int_to_float_cast_ops(%arg0: i8, %arg1: i64) { // ----- -func.func @arith_cmpi_eq(%arg0: i32, %arg1: i32) -> i1 { - // CHECK-LABEL: arith_cmpi_eq - // CHECK-SAME: ([[Arg0:[^ ]*]]: i32, [[Arg1:[^ ]*]]: i32) - // CHECK-DAG: [[EQ:[^ ]*]] = emitc.cmp eq, [[Arg0]], [[Arg1]] : (i32, i32) -> i1 - %eq = arith.cmpi eq, %arg0, %arg1 : i32 - // CHECK: return [[EQ]] - return %eq: i1 -} - -func.func @arith_cmpi_ult(%arg0: i32, %arg1: i32) -> i1 { - // CHECK-LABEL: arith_cmpi_ult - // CHECK-SAME: ([[Arg0:[^ ]*]]: i32, [[Arg1:[^ ]*]]: i32) - // CHECK-DAG: [[CastArg0:[^ ]*]] = emitc.cast [[Arg0]] : i32 to ui32 - // CHECK-DAG: [[CastArg1:[^ ]*]] = emitc.cast [[Arg1]] : i32 to ui32 - // CHECK-DAG: [[ULT:[^ ]*]] = emitc.cmp lt, [[CastArg0]], [[CastArg1]] : (ui32, ui32) -> i1 - %eq = arith.cmpi ult, %arg0, %arg1 : i32 - // CHECK: return [[ULT]] - return %eq: i1 -} - -// ----- - func.func @trunci(%arg0: i32) -> i8 { // CHECK-LABEL: trunci // CHECK-SAME: (%[[Arg0:[^ ]*]]: i32) From 35efde08c34c807db41b79c7cd070e6443486552 Mon Sep 17 00:00:00 2001 From: Corentin Ferry Date: Tue, 7 May 2024 16:18:04 +0100 Subject: [PATCH 8/9] Review comments, remove support for index casts for now --- .../Conversion/ArithToEmitC/ArithToEmitC.cpp | 23 +++++-------- .../arith-to-emitc-unsupported.mlir | 19 +++++++++++ .../ArithToEmitC/arith-to-emitc.mlir | 32 +------------------ 3 files changed, 28 insertions(+), 46 deletions(-) diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp index 6e6fc591ca9ba..947adb277ec57 100644 --- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp +++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp @@ -284,7 +284,7 @@ class CastConversion : public OpConversionPattern { ConversionPatternRewriter &rewriter) const override { Type opReturnType = this->getTypeConverter()->convertType(op.getType()); - if (!isa_and_nonnull(opReturnType)) { + if (!isa_and_nonnull(opReturnType)) { return rewriter.notifyMatchFailure(op, "expected result integer type"); } @@ -293,31 +293,26 @@ class CastConversion : public OpConversionPattern { op, "CastConversion only supports unary ops"); } - Type operandType = - this->getTypeConverter()->convertType(adaptor.getIn().getType()); - if (!isa_and_nonnull(operandType)) { + Type operandType = adaptor.getIn().getType(); + if (!isa_and_nonnull(operandType)) { return rewriter.notifyMatchFailure(op, "expected operand integer type"); } - bool doUnsigned = - needsUnsigned || - (isa(operandType) && isa(opReturnType) && - operandType.getIntOrFloatBitWidth() > - opReturnType.getIntOrFloatBitWidth()); + bool isTruncation = operandType.getIntOrFloatBitWidth() > + opReturnType.getIntOrFloatBitWidth(); + bool doUnsigned = needsUnsigned || isTruncation; Type castType = opReturnType; // For int conversions: if the op is a ui variant and the type wanted as // return type isn't unsigned, we need to issue an unsigned type to do // the conversion. - if (isa(opReturnType) && - castType.isUnsignedInteger() != doUnsigned) + if (castType.isUnsignedInteger() != doUnsigned) castType = rewriter.getIntegerType(opReturnType.getIntOrFloatBitWidth(), /*isSigned=*/!doUnsigned); Value actualOp = adaptor.getIn(); // Fix the signedness of the operand if necessary - if (isa(operandType) && - operandType.isUnsignedInteger() != doUnsigned) { + if (operandType.isUnsignedInteger() != doUnsigned) { Type correctSignednessType = rewriter.getIntegerType(operandType.getIntOrFloatBitWidth(), /*isSigned=*/!doUnsigned); @@ -558,8 +553,6 @@ void mlir::populateArithToEmitCPatterns(TypeConverter &typeConverter, UnsignedCastConversion, SignedCastConversion, UnsignedCastConversion, - SignedCastConversion, - UnsignedCastConversion, ItoFCastOpConversion, ItoFCastOpConversion, FtoICastOpConversion, diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir index 32c0c0381d326..5fcb2b3a553e5 100644 --- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir +++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir @@ -79,3 +79,22 @@ func.func @arith_cast_fptoui_i1(%arg0: f32) -> i1 { return %t: i1 } +// ----- + +func.func @index_cast(%arg0: i32) -> i32 { + // expected-error @+1 {{failed to legalize operation 'arith.index_cast'}} + %idx = arith.index_cast %arg0 : i32 to index + %int = arith.index_cast %idx : index to i32 + + return %int : i32 +} + +// ----- + +func.func @index_castui(%arg0: i32) -> i32 { + // expected-error @+1 {{failed to legalize operation 'arith.index_castui'}} + %idx = arith.index_castui %arg0 : i32 to index + %int = arith.index_castui %idx : index to i32 + + return %int : i32 +} diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir index 0a4d16b132e1d..bda1180282142 100644 --- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir +++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc.mlir @@ -406,36 +406,6 @@ func.func @trunci(%arg0: i32) -> i8 { // ----- -func.func @index_cast(%arg0: i32) -> i32 { - // CHECK-LABEL: index_cast - // CHECK-SAME: ([[Arg0:[^ ]*]]: i32) - // CHECK: %[[ICast:.*]] = emitc.cast [[Arg0]] : i32 to index - // CHECK: emitc.cast %[[ICast]] : index to i32 - - %idx = arith.index_cast %arg0 : i32 to index - %int = arith.index_cast %idx : index to i32 - - return %int : i32 -} - -// ----- - -func.func @index_castui(%arg0: i32) -> i32 { - // CHECK-LABEL: index_castui - // CHECK-SAME: (%[[Arg0:[^ ]*]]: i32) - // CHECK: %[[IToUI:.*]] = emitc.cast %[[Arg0]] : i32 to ui32 - // CHECK: %[[IdxCast:.*]] = emitc.cast %[[IToUI]] : ui32 to index - // CHECK: %[[UIntCast:.*]] = emitc.cast %[[IdxCast]] : index to ui32 - // CHECK: emitc.cast %[[UIntCast]] : ui32 to i32 - - %idx = arith.index_castui %arg0 : i32 to index - %int = arith.index_castui %idx : index to i32 - - return %int : i32 -} - -// ----- - func.func @extsi(%arg0: i32) { // CHECK-LABEL: extsi // CHECK-SAME: ([[Arg0:[^ ]*]]: i32) @@ -458,4 +428,4 @@ func.func @extui(%arg0: i32) { %extd = arith.extui %arg0 : i32 to i64 return -} \ No newline at end of file +} From 3892a757cb5e05707cf7e180077e0e01a1c9db90 Mon Sep 17 00:00:00 2001 From: Corentin Ferry Date: Wed, 8 May 2024 09:51:50 +0100 Subject: [PATCH 9/9] Review comments --- mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp index 947adb277ec57..5fe4fc8695017 100644 --- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp +++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp @@ -285,17 +285,17 @@ class CastConversion : public OpConversionPattern { Type opReturnType = this->getTypeConverter()->convertType(op.getType()); if (!isa_and_nonnull(opReturnType)) { - return rewriter.notifyMatchFailure(op, "expected result integer type"); + return rewriter.notifyMatchFailure(op, "expected integer result type"); } - if (adaptor.getOperands().size() > 1) { + if (adaptor.getOperands().size() != 1) { return rewriter.notifyMatchFailure( op, "CastConversion only supports unary ops"); } Type operandType = adaptor.getIn().getType(); if (!isa_and_nonnull(operandType)) { - return rewriter.notifyMatchFailure(op, "expected operand integer type"); + return rewriter.notifyMatchFailure(op, "expected integer operand type"); } bool isTruncation = operandType.getIntOrFloatBitWidth() > @@ -306,9 +306,10 @@ class CastConversion : public OpConversionPattern { // For int conversions: if the op is a ui variant and the type wanted as // return type isn't unsigned, we need to issue an unsigned type to do // the conversion. - if (castType.isUnsignedInteger() != doUnsigned) + if (castType.isUnsignedInteger() != doUnsigned) { castType = rewriter.getIntegerType(opReturnType.getIntOrFloatBitWidth(), /*isSigned=*/!doUnsigned); + } Value actualOp = adaptor.getIn(); // Fix the signedness of the operand if necessary