From ca055daee31c4753750a522c89a6c1602581a2c7 Mon Sep 17 00:00:00 2001 From: Jose Lopes Date: Tue, 19 Nov 2024 14:21:30 +0000 Subject: [PATCH 1/8] Fix conversion --- mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp | 55 +++++++++++++------ mlir/test/Conversion/SCFToEmitC/for.mlir | 52 +++++++++++++++--- mlir/test/Conversion/SCFToEmitC/if.mlir | 26 +++++++++ 3 files changed, 110 insertions(+), 23 deletions(-) diff --git a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp index 51490c79ce490..ce57b5389aad9 100644 --- a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp +++ b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp @@ -21,6 +21,7 @@ #include "mlir/IR/IRMapping.h" #include "mlir/IR/MLIRContext.h" #include "mlir/IR/PatternMatch.h" +#include "mlir/IR/Value.h" #include "mlir/Transforms/DialectConversion.h" #include "mlir/Transforms/OneToNTypeConversion.h" #include "mlir/Transforms/Passes.h" @@ -79,22 +80,36 @@ createVariablesForResults(T op, const TypeConverter *typeConverter, // Create a series of assign ops assigning given values to given variables at // the current insertion point of given rewriter. -static void assignValues(ValueRange values, SmallVector &variables, - ConversionPatternRewriter &rewriter, Location loc) { +static void assignValues(ValueRange values, ValueRange variables, + ConversionPatternRewriter &rewriter, Location loc, + const TypeConverter *typeConverter = nullptr) { for (auto [value, var] : llvm::zip(values, variables)) rewriter.create(loc, var, value); } -static void lowerYield(SmallVector &resultVariables, - ConversionPatternRewriter &rewriter, - scf::YieldOp yield) { +static void lowerYield(ValueRange resultVariables, + ConversionPatternRewriter &rewriter, scf::YieldOp yield, + const TypeConverter *typeConverter) { Location loc = yield.getLoc(); - ValueRange operands = yield.getOperands(); OpBuilder::InsertionGuard guard(rewriter); rewriter.setInsertionPoint(yield); - assignValues(operands, resultVariables, rewriter, loc); + SmallVector yieldOperands; + for (auto originalOperand : yield.getOperands()) { + Value operand = originalOperand; + + if (typeConverter && !typeConverter->isLegal(operand.getType())) { + Type resultType = typeConverter->convertType(operand.getType()); + auto castToTarget = + rewriter.create(loc, resultType, operand); + operand = castToTarget.getResult(0); + } + + yieldOperands.push_back(operand); + } + + assignValues(yieldOperands, resultVariables, rewriter, loc); rewriter.create(loc); rewriter.eraseOp(yield); @@ -118,22 +133,29 @@ ForLowering::matchAndRewrite(ForOp forOp, OpAdaptor adaptor, emitc::ForOp loweredFor = rewriter.create( loc, adaptor.getLowerBound(), adaptor.getUpperBound(), adaptor.getStep()); - // Propagate any attributes from the ODS forOp to the lowered emitc::for op. - loweredFor->setAttrs(forOp->getAttrs()); - Block *loweredBody = loweredFor.getBody(); // Erase the auto-generated terminator for the lowered for op. rewriter.eraseOp(loweredBody->getTerminator()); + // Convert the original region types into the new types by adding unrealized + // casts in the begginning of the loop. This performs the conversion in place. + if (failed(rewriter.convertRegionTypes(&forOp.getRegion(), + *getTypeConverter(), nullptr))) { + return rewriter.notifyMatchFailure(forOp, "region types conversion failed"); + } + + // Register the replacements for the block arguments and inline the body of + // the scf.for loop into the body of the emitc::for loop. + Block *scfBody = &(forOp.getRegion().front()); SmallVector replacingValues; replacingValues.push_back(loweredFor.getInductionVar()); replacingValues.append(resultVariables.begin(), resultVariables.end()); + rewriter.mergeBlocks(scfBody, loweredBody, replacingValues); - Block *adaptorBody = &(adaptor.getRegion().front()); - rewriter.mergeBlocks(adaptorBody, loweredBody, replacingValues); lowerYield(resultVariables, rewriter, - cast(loweredBody->getTerminator())); + cast(loweredBody->getTerminator()), + getTypeConverter()); rewriter.replaceOp(forOp, resultVariables); return success(); @@ -169,11 +191,12 @@ IfLowering::matchAndRewrite(IfOp ifOp, OpAdaptor adaptor, // emitc::if regions, but the scf::yield is replaced not only with an // emitc::yield, but also with a sequence of emitc::assign ops that set the // yielded values into the result variables. - auto lowerRegion = [&resultVariables, &rewriter](Region ®ion, - Region &loweredRegion) { + auto lowerRegion = [&resultVariables, &rewriter, + this](Region ®ion, Region &loweredRegion) { rewriter.inlineRegionBefore(region, loweredRegion, loweredRegion.end()); Operation *terminator = loweredRegion.back().getTerminator(); - lowerYield(resultVariables, rewriter, cast(terminator)); + lowerYield(resultVariables, rewriter, cast(terminator), + getTypeConverter()); }; Region &thenRegion = adaptor.getThenRegion(); diff --git a/mlir/test/Conversion/SCFToEmitC/for.mlir b/mlir/test/Conversion/SCFToEmitC/for.mlir index b422aaa4545d9..9a0468b7d37cd 100644 --- a/mlir/test/Conversion/SCFToEmitC/for.mlir +++ b/mlir/test/Conversion/SCFToEmitC/for.mlir @@ -99,11 +99,49 @@ func.func @nested_for_yield(%arg0 : index, %arg1 : index, %arg2 : index) -> f32 // CHECK-NEXT: return %[[VAL_4]] : f32 // CHECK-NEXT: } -func.func @loop_with_attr(%arg0 : index, %arg1 : index, %arg2 : index) { - scf.for %i0 = %arg0 to %arg1 step %arg2 { - %c1 = arith.constant 1 : index - } {test.value = 5 : index} - return +func.func @for_yield_index(%arg0 : index, %arg1 : index, %arg2 : index) -> index { + %zero = arith.constant 0 : index + %r = scf.for %i0 = %arg0 to %arg1 step %arg2 iter_args(%acc = %zero) -> index { + scf.yield %acc : index + } + return %r : index } -// CHECK-LABEL: func.func @loop_with_attr -// CHECK: {test.value = 5 : index} + +// CHECK: func.func @for_yield_index(%arg0: index, %arg1: index, %arg2: index) -> index { +// CHECK: %0 = builtin.unrealized_conversion_cast %arg2 : index to !emitc.size_t +// CHECK: %1 = builtin.unrealized_conversion_cast %arg1 : index to !emitc.size_t +// CHECK: %2 = builtin.unrealized_conversion_cast %arg0 : index to !emitc.size_t +// CHECK: %c0 = arith.constant 0 : index +// CHECK: %3 = builtin.unrealized_conversion_cast %c0 : index to !emitc.size_t +// CHECK: %4 = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t +// CHECK: emitc.assign %3 : !emitc.size_t to %4 : !emitc.size_t +// CHECK: emitc.for %arg3 = %2 to %1 step %0 { +// CHECK: %6 = builtin.unrealized_conversion_cast %4 : !emitc.size_t to index +// CHECK: %7 = builtin.unrealized_conversion_cast %6 : index to !emitc.size_t +// CHECK: emitc.assign %7 : !emitc.size_t to %4 : !emitc.size_t +// CHECK: } +// CHECK: %5 = builtin.unrealized_conversion_cast %4 : !emitc.size_t to index +// CHECK: return %5 : index +// CHECK: } + + + func.func @for_yield_i32(%arg0 : index, %arg1 : index, %arg2 : index) -> i32 { + %zero = arith.constant 0 : i32 + %r = scf.for %i0 = %arg0 to %arg1 step %arg2 iter_args(%acc = %zero) -> i32 { + scf.yield %acc : i32 + } + return %r : i32 + } + +// CHECK: func.func @for_yield_i32(%arg0: index, %arg1: index, %arg2: index) -> i32 { +// CHECK: %0 = builtin.unrealized_conversion_cast %arg2 : index to !emitc.size_t +// CHECK: %1 = builtin.unrealized_conversion_cast %arg1 : index to !emitc.size_t +// CHECK: %2 = builtin.unrealized_conversion_cast %arg0 : index to !emitc.size_t +// CHECK: %c0_i32 = arith.constant 0 : i32 +// CHECK: %3 = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> i32 +// CHECK: emitc.assign %c0_i32 : i32 to %3 : i32 +// CHECK: emitc.for %arg3 = %2 to %1 step %0 { +// CHECK: emitc.assign %3 : i32 to %3 : i32 +// CHECK: } +// CHECK: return %3 : i32 +// CHECK: } diff --git a/mlir/test/Conversion/SCFToEmitC/if.mlir b/mlir/test/Conversion/SCFToEmitC/if.mlir index afc9abc761eb4..3a7e55d262479 100644 --- a/mlir/test/Conversion/SCFToEmitC/if.mlir +++ b/mlir/test/Conversion/SCFToEmitC/if.mlir @@ -68,3 +68,29 @@ func.func @test_if_yield(%arg0: i1, %arg1: f32) { // CHECK-NEXT: } // CHECK-NEXT: return // CHECK-NEXT: } + + +func.func @test_if_yield_index(%arg0: i1, %arg1: f32) { + %0 = arith.constant 0 : index + %1 = arith.constant 0 : index + %x = scf.if %arg0 -> (index) { + scf.yield %0 : index + } else { + scf.yield %1 : index + } + return +} + +// CHECK:func.func @test_if_yield_index(%arg0: i1, %arg1: f32) { +// CHECK: %c0 = arith.constant 0 : index +// CHECK: %c0_0 = arith.constant 0 : index +// CHECK: %0 = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t +// CHECK: emitc.if %arg0 { +// CHECK: %1 = builtin.unrealized_conversion_cast %c0 : index to !emitc.size_t +// CHECK: emitc.assign %1 : !emitc.size_t to %0 : !emitc.size_t +// CHECK: } else { +// CHECK: %1 = builtin.unrealized_conversion_cast %c0_0 : index to !emitc.size_t +// CHECK: emitc.assign %1 : !emitc.size_t to %0 : !emitc.size_t +// CHECK: } +// CHECK: return +// CHECK: } From ed7f19e31d9008ed1a2c490dda425e58bc0f21ac Mon Sep 17 00:00:00 2001 From: Jose Lopes Date: Wed, 20 Nov 2024 12:46:35 +0000 Subject: [PATCH 2/8] Add extra test --- mlir/test/Conversion/SCFToEmitC/for.mlir | 30 +++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/mlir/test/Conversion/SCFToEmitC/for.mlir b/mlir/test/Conversion/SCFToEmitC/for.mlir index 9a0468b7d37cd..902ce0885b3c3 100644 --- a/mlir/test/Conversion/SCFToEmitC/for.mlir +++ b/mlir/test/Conversion/SCFToEmitC/for.mlir @@ -125,7 +125,7 @@ func.func @for_yield_index(%arg0 : index, %arg1 : index, %arg2 : index) -> index // CHECK: } - func.func @for_yield_i32(%arg0 : index, %arg1 : index, %arg2 : index) -> i32 { +func.func @for_yield_i32(%arg0 : index, %arg1 : index, %arg2 : index) -> i32 { %zero = arith.constant 0 : i32 %r = scf.for %i0 = %arg0 to %arg1 step %arg2 iter_args(%acc = %zero) -> i32 { scf.yield %acc : i32 @@ -145,3 +145,31 @@ func.func @for_yield_index(%arg0 : index, %arg1 : index, %arg2 : index) -> index // CHECK: } // CHECK: return %3 : i32 // CHECK: } + + +func.func @for_yield_update_loop_carried_var(%arg0 : index, %arg1 : index, %arg2 : index) -> index { + %zero = arith.constant 0 : index + %r = scf.for %i0 = %arg0 to %arg1 step %arg2 iter_args(%acc = %zero) -> index { + %sn = arith.addi %acc, %acc : index + scf.yield %sn: index + } + return %r : index + } + +// CHECK: func.func @for_yield_update_loop_carried_var(%arg0: index, %arg1: index, %arg2: index) -> index { +// CHECK: %0 = builtin.unrealized_conversion_cast %arg2 : index to !emitc.size_t +// CHECK: %1 = builtin.unrealized_conversion_cast %arg1 : index to !emitc.size_t +// CHECK: %2 = builtin.unrealized_conversion_cast %arg0 : index to !emitc.size_t +// CHECK: %c0 = arith.constant 0 : index +// CHECK: %3 = builtin.unrealized_conversion_cast %c0 : index to !emitc.size_t +// CHECK: %4 = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t +// CHECK: emitc.assign %3 : !emitc.size_t to %4 : !emitc.size_t +// CHECK: emitc.for %arg3 = %2 to %1 step %0 { +// CHECK: %6 = builtin.unrealized_conversion_cast %4 : !emitc.size_t to index +// CHECK: %7 = arith.addi %6, %6 : index +// CHECK: %8 = builtin.unrealized_conversion_cast %7 : index to !emitc.size_t +// CHECK: emitc.assign %8 : !emitc.size_t to %4 : !emitc.size_t +// CHECK: } +// CHECK: %5 = builtin.unrealized_conversion_cast %4 : !emitc.size_t to index +// CHECK: return %5 : index +// CHECK: } From ff550903227e9ada7ef9b74b4abf9be96348db7b Mon Sep 17 00:00:00 2001 From: Jose Lopes Date: Wed, 20 Nov 2024 12:47:53 +0000 Subject: [PATCH 3/8] Remove smoke test --- mlir/test/Conversion/SCFToEmitC/for.mlir | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/mlir/test/Conversion/SCFToEmitC/for.mlir b/mlir/test/Conversion/SCFToEmitC/for.mlir index 902ce0885b3c3..cf2ef21476fe1 100644 --- a/mlir/test/Conversion/SCFToEmitC/for.mlir +++ b/mlir/test/Conversion/SCFToEmitC/for.mlir @@ -125,28 +125,6 @@ func.func @for_yield_index(%arg0 : index, %arg1 : index, %arg2 : index) -> index // CHECK: } -func.func @for_yield_i32(%arg0 : index, %arg1 : index, %arg2 : index) -> i32 { - %zero = arith.constant 0 : i32 - %r = scf.for %i0 = %arg0 to %arg1 step %arg2 iter_args(%acc = %zero) -> i32 { - scf.yield %acc : i32 - } - return %r : i32 - } - -// CHECK: func.func @for_yield_i32(%arg0: index, %arg1: index, %arg2: index) -> i32 { -// CHECK: %0 = builtin.unrealized_conversion_cast %arg2 : index to !emitc.size_t -// CHECK: %1 = builtin.unrealized_conversion_cast %arg1 : index to !emitc.size_t -// CHECK: %2 = builtin.unrealized_conversion_cast %arg0 : index to !emitc.size_t -// CHECK: %c0_i32 = arith.constant 0 : i32 -// CHECK: %3 = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> i32 -// CHECK: emitc.assign %c0_i32 : i32 to %3 : i32 -// CHECK: emitc.for %arg3 = %2 to %1 step %0 { -// CHECK: emitc.assign %3 : i32 to %3 : i32 -// CHECK: } -// CHECK: return %3 : i32 -// CHECK: } - - func.func @for_yield_update_loop_carried_var(%arg0 : index, %arg1 : index, %arg2 : index) -> index { %zero = arith.constant 0 : index %r = scf.for %i0 = %arg0 to %arg1 step %arg2 iter_args(%acc = %zero) -> index { From 888fe3789978f5e8a8ca399791f79a6bd5775e71 Mon Sep 17 00:00:00 2001 From: Jose Lopes Date: Wed, 20 Nov 2024 14:46:08 +0000 Subject: [PATCH 4/8] Beautify tests --- mlir/test/Conversion/SCFToEmitC/for.mlir | 60 ++++++++++++------------ mlir/test/Conversion/SCFToEmitC/if.mlir | 21 +++++---- 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/mlir/test/Conversion/SCFToEmitC/for.mlir b/mlir/test/Conversion/SCFToEmitC/for.mlir index cf2ef21476fe1..9f12e1f77aa05 100644 --- a/mlir/test/Conversion/SCFToEmitC/for.mlir +++ b/mlir/test/Conversion/SCFToEmitC/for.mlir @@ -107,21 +107,22 @@ func.func @for_yield_index(%arg0 : index, %arg1 : index, %arg2 : index) -> index return %r : index } -// CHECK: func.func @for_yield_index(%arg0: index, %arg1: index, %arg2: index) -> index { -// CHECK: %0 = builtin.unrealized_conversion_cast %arg2 : index to !emitc.size_t -// CHECK: %1 = builtin.unrealized_conversion_cast %arg1 : index to !emitc.size_t -// CHECK: %2 = builtin.unrealized_conversion_cast %arg0 : index to !emitc.size_t -// CHECK: %c0 = arith.constant 0 : index -// CHECK: %3 = builtin.unrealized_conversion_cast %c0 : index to !emitc.size_t -// CHECK: %4 = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t -// CHECK: emitc.assign %3 : !emitc.size_t to %4 : !emitc.size_t -// CHECK: emitc.for %arg3 = %2 to %1 step %0 { -// CHECK: %6 = builtin.unrealized_conversion_cast %4 : !emitc.size_t to index -// CHECK: %7 = builtin.unrealized_conversion_cast %6 : index to !emitc.size_t -// CHECK: emitc.assign %7 : !emitc.size_t to %4 : !emitc.size_t +// CHECK-LABEL: func.func @for_yield_index( +// CHECK-SAME: %[[ARG_0:.*]]: index, %[[ARG_1:.*]]: index, %[[ARG_2:.*]]: index) -> index { +// CHECK: %[[VAL_0:.*]] = builtin.unrealized_conversion_cast %[[ARG_2]] : index to !emitc.size_t +// CHECK: %[[VAL_1:.*]] = builtin.unrealized_conversion_cast %[[ARG_1]] : index to !emitc.size_t +// CHECK: %[[VAL_2:.*]] = builtin.unrealized_conversion_cast %[[ARG_0]] : index to !emitc.size_t +// CHECK: %[[C0:.*]] = arith.constant 0 : index +// CHECK: %[[VAL_3:.*]] = builtin.unrealized_conversion_cast %[[C0]] : index to !emitc.size_t +// CHECK: %[[VAL_4:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t +// CHECK: emitc.assign %[[VAL_3]] : !emitc.size_t to %[[VAL_4]] : !emitc.size_t +// CHECK: emitc.for %[[VAL_5:.*]] = %[[VAL_2]] to %[[VAL_1]] step %[[VAL_0]] { +// CHECK: %[[VAL_6:.*]] = builtin.unrealized_conversion_cast %[[VAL_4]] : !emitc.size_t to index +// CHECK: %[[VAL_7:.*]] = builtin.unrealized_conversion_cast %[[VAL_6]] : index to !emitc.size_t +// CHECK: emitc.assign %[[VAL_7]] : !emitc.size_t to %[[VAL_4]] : !emitc.size_t // CHECK: } -// CHECK: %5 = builtin.unrealized_conversion_cast %4 : !emitc.size_t to index -// CHECK: return %5 : index +// CHECK: %[[VAL_8:.*]] = builtin.unrealized_conversion_cast %[[VAL_4]] : !emitc.size_t to index +// CHECK: return %[[VAL_8]] : index // CHECK: } @@ -134,20 +135,21 @@ func.func @for_yield_update_loop_carried_var(%arg0 : index, %arg1 : index, %arg2 return %r : index } -// CHECK: func.func @for_yield_update_loop_carried_var(%arg0: index, %arg1: index, %arg2: index) -> index { -// CHECK: %0 = builtin.unrealized_conversion_cast %arg2 : index to !emitc.size_t -// CHECK: %1 = builtin.unrealized_conversion_cast %arg1 : index to !emitc.size_t -// CHECK: %2 = builtin.unrealized_conversion_cast %arg0 : index to !emitc.size_t -// CHECK: %c0 = arith.constant 0 : index -// CHECK: %3 = builtin.unrealized_conversion_cast %c0 : index to !emitc.size_t -// CHECK: %4 = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t -// CHECK: emitc.assign %3 : !emitc.size_t to %4 : !emitc.size_t -// CHECK: emitc.for %arg3 = %2 to %1 step %0 { -// CHECK: %6 = builtin.unrealized_conversion_cast %4 : !emitc.size_t to index -// CHECK: %7 = arith.addi %6, %6 : index -// CHECK: %8 = builtin.unrealized_conversion_cast %7 : index to !emitc.size_t -// CHECK: emitc.assign %8 : !emitc.size_t to %4 : !emitc.size_t +// CHECK-LABEL: func.func @for_yield_update_loop_carried_var( +// CHECK-SAME: %[[ARG_0:.*]]: index, %[[ARG_1:.*]]: index, %[[ARG_2:.*]]: index) -> index { +// CHECK: %[[VAL_0:.*]] = builtin.unrealized_conversion_cast %[[ARG_2]] : index to !emitc.size_t +// CHECK: %[[VAL_1:.*]] = builtin.unrealized_conversion_cast %[[ARG_1]] : index to !emitc.size_t +// CHECK: %[[VAL_2:.*]] = builtin.unrealized_conversion_cast %[[ARG_0]] : index to !emitc.size_t +// CHECK: %[[C0:.*]] = arith.constant 0 : index +// CHECK: %[[VAL_3:.*]] = builtin.unrealized_conversion_cast %[[C0]] : index to !emitc.size_t +// CHECK: %[[VAL_4:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t +// CHECK: emitc.assign %[[VAL_3]] : !emitc.size_t to %[[VAL_4]] : !emitc.size_t +// CHECK: emitc.for %[[ARG_3:.*]] = %[[VAL_2]] to %[[VAL_1]] step %[[VAL_0]] { +// CHECK: %[[VAL_5:.*]] = builtin.unrealized_conversion_cast %[[VAL_4]] : !emitc.size_t to index +// CHECK: %[[VAL_6:.*]] = arith.addi %[[VAL_5]], %[[VAL_5]] : index +// CHECK: %[[VAL_8:.*]] = builtin.unrealized_conversion_cast %[[VAL_6]] : index to !emitc.size_t +// CHECK: emitc.assign %[[VAL_8]] : !emitc.size_t to %[[VAL_4]] : !emitc.size_t // CHECK: } -// CHECK: %5 = builtin.unrealized_conversion_cast %4 : !emitc.size_t to index -// CHECK: return %5 : index +// CHECK: %[[VAL_9:.*]] = builtin.unrealized_conversion_cast %[[VAL_4]] : !emitc.size_t to index +// CHECK: return %[[VAL_9]] : index // CHECK: } diff --git a/mlir/test/Conversion/SCFToEmitC/if.mlir b/mlir/test/Conversion/SCFToEmitC/if.mlir index 3a7e55d262479..50622a124e4fe 100644 --- a/mlir/test/Conversion/SCFToEmitC/if.mlir +++ b/mlir/test/Conversion/SCFToEmitC/if.mlir @@ -72,7 +72,7 @@ func.func @test_if_yield(%arg0: i1, %arg1: f32) { func.func @test_if_yield_index(%arg0: i1, %arg1: f32) { %0 = arith.constant 0 : index - %1 = arith.constant 0 : index + %1 = arith.constant 1 : index %x = scf.if %arg0 -> (index) { scf.yield %0 : index } else { @@ -81,16 +81,17 @@ func.func @test_if_yield_index(%arg0: i1, %arg1: f32) { return } -// CHECK:func.func @test_if_yield_index(%arg0: i1, %arg1: f32) { -// CHECK: %c0 = arith.constant 0 : index -// CHECK: %c0_0 = arith.constant 0 : index -// CHECK: %0 = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t -// CHECK: emitc.if %arg0 { -// CHECK: %1 = builtin.unrealized_conversion_cast %c0 : index to !emitc.size_t -// CHECK: emitc.assign %1 : !emitc.size_t to %0 : !emitc.size_t +// CHECK: func.func @test_if_yield_index( +// CHECK-SAME: %[[ARG_0:.*]]: i1, %[[ARG_1:.*]]: f32) { +// CHECK: %[[C0:.*]] = arith.constant 0 : index +// CHECK: %[[C1:.*]] = arith.constant 1 : index +// CHECK: %[[VAL_0:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t +// CHECK: emitc.if %[[ARG_0]] { +// CHECK: %[[VAL_1:.*]] = builtin.unrealized_conversion_cast %[[C0]] : index to !emitc.size_t +// CHECK: emitc.assign %[[VAL_1]] : !emitc.size_t to %[[VAL_0]] : !emitc.size_t // CHECK: } else { -// CHECK: %1 = builtin.unrealized_conversion_cast %c0_0 : index to !emitc.size_t -// CHECK: emitc.assign %1 : !emitc.size_t to %0 : !emitc.size_t +// CHECK: %[[VAL_2:.*]] = builtin.unrealized_conversion_cast %[[C1]] : index to !emitc.size_t +// CHECK: emitc.assign %[[VAL_2]] : !emitc.size_t to %[[VAL_0]] : !emitc.size_t // CHECK: } // CHECK: return // CHECK: } From 5ed59a8f3670a69c19e005ea8ebc31dfcb5803a3 Mon Sep 17 00:00:00 2001 From: Jose Lopes Date: Wed, 20 Nov 2024 14:51:04 +0000 Subject: [PATCH 5/8] Fix typo --- mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp index ce57b5389aad9..1edd3f5718a27 100644 --- a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp +++ b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp @@ -139,7 +139,7 @@ ForLowering::matchAndRewrite(ForOp forOp, OpAdaptor adaptor, rewriter.eraseOp(loweredBody->getTerminator()); // Convert the original region types into the new types by adding unrealized - // casts in the begginning of the loop. This performs the conversion in place. + // casts in the beginning of the loop. This performs the conversion in place. if (failed(rewriter.convertRegionTypes(&forOp.getRegion(), *getTypeConverter(), nullptr))) { return rewriter.notifyMatchFailure(forOp, "region types conversion failed"); From bb66650b1f61615781faebb4280ba3b54c5f1928 Mon Sep 17 00:00:00 2001 From: Jose Lopes Date: Thu, 21 Nov 2024 10:08:11 +0000 Subject: [PATCH 6/8] Use delayed conversion --- mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp | 54 +++++++++++-------- mlir/test/Conversion/SCFToEmitC/for.mlir | 4 +- mlir/test/Conversion/SCFToEmitC/if.mlir | 10 ++-- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp index 1edd3f5718a27..d5dd14aae46ca 100644 --- a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp +++ b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp @@ -21,7 +21,6 @@ #include "mlir/IR/IRMapping.h" #include "mlir/IR/MLIRContext.h" #include "mlir/IR/PatternMatch.h" -#include "mlir/IR/Value.h" #include "mlir/Transforms/DialectConversion.h" #include "mlir/Transforms/OneToNTypeConversion.h" #include "mlir/Transforms/Passes.h" @@ -81,15 +80,14 @@ createVariablesForResults(T op, const TypeConverter *typeConverter, // Create a series of assign ops assigning given values to given variables at // the current insertion point of given rewriter. static void assignValues(ValueRange values, ValueRange variables, - ConversionPatternRewriter &rewriter, Location loc, - const TypeConverter *typeConverter = nullptr) { + ConversionPatternRewriter &rewriter, Location loc) { for (auto [value, var] : llvm::zip(values, variables)) rewriter.create(loc, var, value); } -static void lowerYield(ValueRange resultVariables, - ConversionPatternRewriter &rewriter, scf::YieldOp yield, - const TypeConverter *typeConverter) { +static LogicalResult lowerYield(Operation *op, ValueRange resultVariables, + ConversionPatternRewriter &rewriter, + scf::YieldOp yield) { Location loc = yield.getLoc(); OpBuilder::InsertionGuard guard(rewriter); @@ -97,22 +95,19 @@ static void lowerYield(ValueRange resultVariables, SmallVector yieldOperands; for (auto originalOperand : yield.getOperands()) { - Value operand = originalOperand; - - if (typeConverter && !typeConverter->isLegal(operand.getType())) { - Type resultType = typeConverter->convertType(operand.getType()); - auto castToTarget = - rewriter.create(loc, resultType, operand); - operand = castToTarget.getResult(0); + Value remappedValue = rewriter.getRemappedValue(originalOperand); + if (!remappedValue) { + return rewriter.notifyMatchFailure(op, "failed to lower yield operands"); } - - yieldOperands.push_back(operand); + yieldOperands.push_back(remappedValue); } assignValues(yieldOperands, resultVariables, rewriter, loc); rewriter.create(loc); rewriter.eraseOp(yield); + + return success(); } LogicalResult @@ -153,9 +148,12 @@ ForLowering::matchAndRewrite(ForOp forOp, OpAdaptor adaptor, replacingValues.append(resultVariables.begin(), resultVariables.end()); rewriter.mergeBlocks(scfBody, loweredBody, replacingValues); - lowerYield(resultVariables, rewriter, - cast(loweredBody->getTerminator()), - getTypeConverter()); + auto result = lowerYield(forOp, resultVariables, rewriter, + cast(loweredBody->getTerminator())); + + if (failed(result)) { + return result; + } rewriter.replaceOp(forOp, resultVariables); return success(); @@ -192,11 +190,15 @@ IfLowering::matchAndRewrite(IfOp ifOp, OpAdaptor adaptor, // emitc::yield, but also with a sequence of emitc::assign ops that set the // yielded values into the result variables. auto lowerRegion = [&resultVariables, &rewriter, - this](Region ®ion, Region &loweredRegion) { + &ifOp](Region ®ion, Region &loweredRegion) { rewriter.inlineRegionBefore(region, loweredRegion, loweredRegion.end()); Operation *terminator = loweredRegion.back().getTerminator(); - lowerYield(resultVariables, rewriter, cast(terminator), - getTypeConverter()); + auto result = lowerYield(ifOp, resultVariables, rewriter, + cast(terminator)); + if (failed(result)) { + return result; + } + return success(); }; Region &thenRegion = adaptor.getThenRegion(); @@ -208,11 +210,17 @@ IfLowering::matchAndRewrite(IfOp ifOp, OpAdaptor adaptor, rewriter.create(loc, adaptor.getCondition(), false, false); Region &loweredThenRegion = loweredIf.getThenRegion(); - lowerRegion(thenRegion, loweredThenRegion); + auto result = lowerRegion(thenRegion, loweredThenRegion); + if (failed(result)) { + return result; + } if (hasElseBlock) { Region &loweredElseRegion = loweredIf.getElseRegion(); - lowerRegion(elseRegion, loweredElseRegion); + auto result = lowerRegion(elseRegion, loweredElseRegion); + if (failed(result)) { + return result; + } } rewriter.replaceOp(ifOp, resultVariables); diff --git a/mlir/test/Conversion/SCFToEmitC/for.mlir b/mlir/test/Conversion/SCFToEmitC/for.mlir index 9f12e1f77aa05..79a53ec8fd4c0 100644 --- a/mlir/test/Conversion/SCFToEmitC/for.mlir +++ b/mlir/test/Conversion/SCFToEmitC/for.mlir @@ -117,9 +117,7 @@ func.func @for_yield_index(%arg0 : index, %arg1 : index, %arg2 : index) -> index // CHECK: %[[VAL_4:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t // CHECK: emitc.assign %[[VAL_3]] : !emitc.size_t to %[[VAL_4]] : !emitc.size_t // CHECK: emitc.for %[[VAL_5:.*]] = %[[VAL_2]] to %[[VAL_1]] step %[[VAL_0]] { -// CHECK: %[[VAL_6:.*]] = builtin.unrealized_conversion_cast %[[VAL_4]] : !emitc.size_t to index -// CHECK: %[[VAL_7:.*]] = builtin.unrealized_conversion_cast %[[VAL_6]] : index to !emitc.size_t -// CHECK: emitc.assign %[[VAL_7]] : !emitc.size_t to %[[VAL_4]] : !emitc.size_t +// CHECK: emitc.assign %[[VAL_4]] : !emitc.size_t to %[[VAL_4]] : !emitc.size_t // CHECK: } // CHECK: %[[VAL_8:.*]] = builtin.unrealized_conversion_cast %[[VAL_4]] : !emitc.size_t to index // CHECK: return %[[VAL_8]] : index diff --git a/mlir/test/Conversion/SCFToEmitC/if.mlir b/mlir/test/Conversion/SCFToEmitC/if.mlir index 50622a124e4fe..eba1dda213e70 100644 --- a/mlir/test/Conversion/SCFToEmitC/if.mlir +++ b/mlir/test/Conversion/SCFToEmitC/if.mlir @@ -84,14 +84,14 @@ func.func @test_if_yield_index(%arg0: i1, %arg1: f32) { // CHECK: func.func @test_if_yield_index( // CHECK-SAME: %[[ARG_0:.*]]: i1, %[[ARG_1:.*]]: f32) { // CHECK: %[[C0:.*]] = arith.constant 0 : index +// CHECK: %[[VAL_0:.*]] = builtin.unrealized_conversion_cast %[[C0]] : index to !emitc.size_t // CHECK: %[[C1:.*]] = arith.constant 1 : index -// CHECK: %[[VAL_0:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t +// CHECK: %[[VAL_1:.*]] = builtin.unrealized_conversion_cast %[[C1]] : index to !emitc.size_t +// CHECK: %[[VAL_2:.*]] = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.size_t // CHECK: emitc.if %[[ARG_0]] { -// CHECK: %[[VAL_1:.*]] = builtin.unrealized_conversion_cast %[[C0]] : index to !emitc.size_t -// CHECK: emitc.assign %[[VAL_1]] : !emitc.size_t to %[[VAL_0]] : !emitc.size_t +// CHECK: emitc.assign %[[VAL_0]] : !emitc.size_t to %[[VAL_2]] : !emitc.size_t // CHECK: } else { -// CHECK: %[[VAL_2:.*]] = builtin.unrealized_conversion_cast %[[C1]] : index to !emitc.size_t -// CHECK: emitc.assign %[[VAL_2]] : !emitc.size_t to %[[VAL_0]] : !emitc.size_t +// CHECK: emitc.assign %[[VAL_1]] : !emitc.size_t to %[[VAL_2]] : !emitc.size_t // CHECK: } // CHECK: return // CHECK: } From f5a954fa342bc1776bb718e3b678067139bafd7c Mon Sep 17 00:00:00 2001 From: Jose Lopes Date: Thu, 21 Nov 2024 10:42:08 +0000 Subject: [PATCH 7/8] Simplify even further --- mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp index d5dd14aae46ca..2568c26d11051 100644 --- a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp +++ b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp @@ -94,12 +94,9 @@ static LogicalResult lowerYield(Operation *op, ValueRange resultVariables, rewriter.setInsertionPoint(yield); SmallVector yieldOperands; - for (auto originalOperand : yield.getOperands()) { - Value remappedValue = rewriter.getRemappedValue(originalOperand); - if (!remappedValue) { - return rewriter.notifyMatchFailure(op, "failed to lower yield operands"); - } - yieldOperands.push_back(remappedValue); + auto result = rewriter.getRemappedValues(yield.getOperands(), yieldOperands); + if (failed(result)) { + return rewriter.notifyMatchFailure(op, "failed to lower yield operands"); } assignValues(yieldOperands, resultVariables, rewriter, loc); From d9fa78fc9f70005b6e1003455eb6e37fac13e50a Mon Sep 17 00:00:00 2001 From: Jose Lopes Date: Thu, 21 Nov 2024 11:52:54 +0000 Subject: [PATCH 8/8] remove auto --- mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp index 2568c26d11051..41c69eed20860 100644 --- a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp +++ b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp @@ -94,8 +94,7 @@ static LogicalResult lowerYield(Operation *op, ValueRange resultVariables, rewriter.setInsertionPoint(yield); SmallVector yieldOperands; - auto result = rewriter.getRemappedValues(yield.getOperands(), yieldOperands); - if (failed(result)) { + if (failed(rewriter.getRemappedValues(yield.getOperands(), yieldOperands))) { return rewriter.notifyMatchFailure(op, "failed to lower yield operands"); }