From d6fe445778556960f1a02dd93f81d4bdfd92482a Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:25:14 +1100 Subject: [PATCH 01/14] Fix --- melior/src/ir/operation/builder.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/melior/src/ir/operation/builder.rs b/melior/src/ir/operation/builder.rs index d45f6bd6ae..91eee5547d 100644 --- a/melior/src/ir/operation/builder.rs +++ b/melior/src/ir/operation/builder.rs @@ -120,7 +120,15 @@ impl<'c> OperationBuilder<'c> { /// Builds an operation. pub fn build(mut self) -> Operation<'c> { - unsafe { Operation::from_raw(mlirOperationCreate(&mut self.raw)) } + unsafe { + let x = mlirOperationCreate(&mut self.raw); + + if x == std::ptr::null() { + panic!() + } + + Operation::from_raw(x) + } } } From 6601e253924297bd8716dfe2046f2b072276335c Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:27:21 +1100 Subject: [PATCH 02/14] Fix --- melior/src/ir/operation/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/melior/src/ir/operation/builder.rs b/melior/src/ir/operation/builder.rs index 91eee5547d..cad175db9d 100644 --- a/melior/src/ir/operation/builder.rs +++ b/melior/src/ir/operation/builder.rs @@ -123,7 +123,7 @@ impl<'c> OperationBuilder<'c> { unsafe { let x = mlirOperationCreate(&mut self.raw); - if x == std::ptr::null() { + if x.ptr.is_null() { panic!() } From f8fda6408a454b34b4acd5b0d94b5855c191c5e6 Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:32:12 +1100 Subject: [PATCH 03/14] Fix --- melior/src/ir/operation.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/melior/src/ir/operation.rs b/melior/src/ir/operation.rs index d93d1450ee..530499dfa0 100644 --- a/melior/src/ir/operation.rs +++ b/melior/src/ir/operation.rs @@ -224,15 +224,7 @@ impl<'c> Operation<'c> { /// Gets the next operation in the same block. pub fn next_in_block(&self) -> Option> { - unsafe { - let operation = mlirOperationGetNextInBlock(self.raw); - - if operation.ptr.is_null() { - None - } else { - Some(OperationRef::from_raw(operation)) - } - } + unsafe { OperationRef::from_option_raw(mlirOperationGetNextInBlock(self.raw)) } } /// Verifies an operation. From daf15642f5498f2eeb0d872dd127a0453e671b3c Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:34:52 +1100 Subject: [PATCH 04/14] Fix --- melior/src/ir/operation/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/melior/src/ir/operation/builder.rs b/melior/src/ir/operation/builder.rs index cad175db9d..e8b643c0d2 100644 --- a/melior/src/ir/operation/builder.rs +++ b/melior/src/ir/operation/builder.rs @@ -124,7 +124,7 @@ impl<'c> OperationBuilder<'c> { let x = mlirOperationCreate(&mut self.raw); if x.ptr.is_null() { - panic!() + panic!("failed to create an operation") } Operation::from_raw(x) From 5ecba9d0435012730269c1f22c2531bdf75f8589 Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:35:12 +1100 Subject: [PATCH 05/14] Fix --- melior/src/ir/operation/builder.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/melior/src/ir/operation/builder.rs b/melior/src/ir/operation/builder.rs index e8b643c0d2..f32bd6a54c 100644 --- a/melior/src/ir/operation/builder.rs +++ b/melior/src/ir/operation/builder.rs @@ -124,6 +124,7 @@ impl<'c> OperationBuilder<'c> { let x = mlirOperationCreate(&mut self.raw); if x.ptr.is_null() { + // TODO Use a result or option type. panic!("failed to create an operation") } From 4024009c17d52e842b423f6cb4cb6a21b84532fe Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:36:24 +1100 Subject: [PATCH 06/14] Fix --- melior/src/ir/operation/builder.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/melior/src/ir/operation/builder.rs b/melior/src/ir/operation/builder.rs index f32bd6a54c..510ea11d50 100644 --- a/melior/src/ir/operation/builder.rs +++ b/melior/src/ir/operation/builder.rs @@ -121,14 +121,14 @@ impl<'c> OperationBuilder<'c> { /// Builds an operation. pub fn build(mut self) -> Operation<'c> { unsafe { - let x = mlirOperationCreate(&mut self.raw); + let operation = mlirOperationCreate(&mut self.raw); - if x.ptr.is_null() { + if operation.ptr.is_null() { // TODO Use a result or option type. panic!("failed to create an operation") } - Operation::from_raw(x) + Operation::from_raw(operation) } } } From f493b9048591cc5cb06a7d966f1762ddeda7a3bf Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:45:11 +1100 Subject: [PATCH 07/14] Fix --- macro/src/dialect/operation/builder.rs | 4 ++-- melior/src/dialect/scf.rs | 7 +++++++ melior/src/ir/operation.rs | 13 +++++++++++++ melior/src/ir/operation/builder.rs | 14 +++----------- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/macro/src/dialect/operation/builder.rs b/macro/src/dialect/operation/builder.rs index 8cf30b7f94..04b3b2696d 100644 --- a/macro/src/dialect/operation/builder.rs +++ b/macro/src/dialect/operation/builder.rs @@ -167,7 +167,7 @@ impl<'o> OperationBuilder<'o> { quote! { impl<'c> #builder_ident<'c, #(#arguments),*> { pub fn build(self) -> #class_name<'c> { - self.builder #maybe_infer.build().try_into().expect(#error) + self.builder #maybe_infer.build().expect("valid operation").try_into().expect(#error) } } } @@ -231,7 +231,7 @@ impl<'o> OperationBuilder<'o> { #[allow(clippy::too_many_arguments)] #[doc = #doc] pub fn #name<'c>(context: &'c ::melior::Context, #(#arguments),*) -> #class_name<'c> { - #class_name::builder(context, location)#(#builder_calls)*.build() + #class_name::builder(context, location)#(#builder_calls)*.build().expect("valid operation") } }) } diff --git a/melior/src/dialect/scf.rs b/melior/src/dialect/scf.rs index 8b3daa1bf5..41519d6af1 100644 --- a/melior/src/dialect/scf.rs +++ b/melior/src/dialect/scf.rs @@ -19,6 +19,7 @@ pub fn condition<'c>( .add_operands(&[condition]) .add_operands(values) .build() + .expect("valid operation") } /// Creates a `scf.execute_region` operation. @@ -32,6 +33,7 @@ pub fn execute_region<'c>( .add_results(result_types) .add_regions(vec![region]) .build() + .expect("valid operation") } /// Creates a `scf.for` operation. @@ -47,6 +49,7 @@ pub fn r#for<'c>( .add_operands(&[start, end, step]) .add_regions(vec![region]) .build() + .expect("valid operation") } /// Creates a `scf.if` operation. @@ -63,6 +66,7 @@ pub fn r#if<'c>( .add_results(result_types) .add_regions(vec![then_region, else_region]) .build() + .expect("valid operation") } /// Creates a `scf.index_switch` operation. @@ -80,6 +84,7 @@ pub fn index_switch<'c>( .add_attributes(&[(Identifier::new(context, "cases"), cases.into())]) .add_regions(regions) .build() + .expect("valid operation") } /// Creates a `scf.while` operation. @@ -96,6 +101,7 @@ pub fn r#while<'c>( .add_results(result_types) .add_regions(vec![before_region, after_region]) .build() + .expect("valid operation") } /// Creates a `scf.yield` operation. @@ -107,6 +113,7 @@ pub fn r#yield<'c>( OperationBuilder::new(context, "scf.yield", location) .add_operands(values) .build() + .expect("valid operation") } #[cfg(test)] diff --git a/melior/src/ir/operation.rs b/melior/src/ir/operation.rs index 530499dfa0..e3efd32a26 100644 --- a/melior/src/ir/operation.rs +++ b/melior/src/ir/operation.rs @@ -267,6 +267,19 @@ impl<'c> Operation<'c> { } } + /// Creates an optional operation from a raw object. + /// + /// # Safety + /// + /// A raw object must be valid. + pub unsafe fn from_option_raw(raw: MlirOperation) -> Option { + if raw.ptr.is_null() { + None + } else { + Some(Self::from_raw(raw)) + } + } + /// Converts an operation into a raw object. pub fn into_raw(self) -> MlirOperation { let operation = self.raw; diff --git a/melior/src/ir/operation/builder.rs b/melior/src/ir/operation/builder.rs index 510ea11d50..64fdfc2af8 100644 --- a/melior/src/ir/operation/builder.rs +++ b/melior/src/ir/operation/builder.rs @@ -119,17 +119,8 @@ impl<'c> OperationBuilder<'c> { } /// Builds an operation. - pub fn build(mut self) -> Operation<'c> { - unsafe { - let operation = mlirOperationCreate(&mut self.raw); - - if operation.ptr.is_null() { - // TODO Use a result or option type. - panic!("failed to create an operation") - } - - Operation::from_raw(operation) - } + pub fn build(mut self) -> Option> { + unsafe { Operation::from_option_raw(mlirOperationCreate(&mut self.raw)) } } } @@ -222,6 +213,7 @@ mod tests { .add_operands(&[argument, argument]) .enable_result_type_inference() .build() + .unwrap() .result(0) .unwrap() .r#type(), From 34fa40007297d537ac9cac5da2882bd7bb201ab1 Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:48:45 +1100 Subject: [PATCH 08/14] Fix --- macro/src/dialect/operation/builder.rs | 2 +- melior/src/lib.rs | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/macro/src/dialect/operation/builder.rs b/macro/src/dialect/operation/builder.rs index 04b3b2696d..f2c4342bf3 100644 --- a/macro/src/dialect/operation/builder.rs +++ b/macro/src/dialect/operation/builder.rs @@ -231,7 +231,7 @@ impl<'o> OperationBuilder<'o> { #[allow(clippy::too_many_arguments)] #[doc = #doc] pub fn #name<'c>(context: &'c ::melior::Context, #(#arguments),*) -> #class_name<'c> { - #class_name::builder(context, location)#(#builder_calls)*.build().expect("valid operation") + #class_name::builder(context, location)#(#builder_calls)*.build() } }) } diff --git a/melior/src/lib.rs b/melior/src/lib.rs index 80f6b46a4b..203f6513bc 100644 --- a/melior/src/lib.rs +++ b/melior/src/lib.rs @@ -135,7 +135,8 @@ mod tests { zero.result(0).unwrap().into(), ]) .add_results(&[index_type]) - .build(), + .build() + .unwrap(), ); let loop_block = Block::new(&[(index_type, location)]); @@ -156,7 +157,8 @@ mod tests { loop_block.argument(0).unwrap().into(), ]) .add_results(&[f32_type]) - .build(), + .build() + .unwrap(), ); let rhs = loop_block.append_operation( @@ -166,7 +168,8 @@ mod tests { loop_block.argument(0).unwrap().into(), ]) .add_results(&[f32_type]) - .build(), + .build() + .unwrap(), ); let add = loop_block.append_operation(arith::addf( @@ -183,7 +186,8 @@ mod tests { function_block.argument(0).unwrap().into(), loop_block.argument(0).unwrap().into(), ]) - .build(), + .build() + .unwrap(), ); loop_block.append_operation(scf::r#yield(&context, &[], location)); From 6e4f3bc60f6bd7bc1d8aee09792f17dae603906c Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:56:03 +1100 Subject: [PATCH 09/14] Fix --- melior/src/dialect/llvm.rs | 22 ++++++++++++++++++++- melior/src/dialect/memref.rs | 16 +++++++++++++--- melior/src/ir/block.rs | 32 +++++++++++++++++++++++-------- melior/src/ir/module.rs | 7 +++++-- melior/src/ir/operation.rs | 27 ++++++++++++++++++++------ melior/src/ir/operation/result.rs | 3 ++- melior/src/ir/value.rs | 22 ++++++++++++++------- 7 files changed, 101 insertions(+), 28 deletions(-) diff --git a/melior/src/dialect/llvm.rs b/melior/src/dialect/llvm.rs index 2b99085360..36b8915437 100644 --- a/melior/src/dialect/llvm.rs +++ b/melior/src/dialect/llvm.rs @@ -35,6 +35,7 @@ pub fn extract_value<'c>( .add_operands(&[container]) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.getelementptr` operation. @@ -60,6 +61,7 @@ pub fn get_element_ptr<'c>( .add_operands(&[ptr]) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.getelementptr` operation with dynamic indices. @@ -86,6 +88,7 @@ pub fn get_element_ptr_dynamic<'c, const N: usize>( .add_operands(indices) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.insertvalue` operation. @@ -101,6 +104,7 @@ pub fn insert_value<'c>( .add_operands(&[container, value]) .enable_result_type_inference() .build() + .expect("valid operation") } /// Creates a `llvm.mlir.undef` operation. @@ -112,6 +116,7 @@ pub fn undef<'c>( OperationBuilder::new(context, "llvm.mlir.undef", location) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.mlir.poison` operation. @@ -123,6 +128,7 @@ pub fn poison<'c>( OperationBuilder::new(context, "llvm.mlir.poison", location) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.mlir.null` operation. A null pointer. @@ -134,6 +140,7 @@ pub fn nullptr<'c>( OperationBuilder::new(context, "llvm.mlir.null", location) .add_results(&[ptr_type]) .build() + .expect("valid operation") } /// Creates a `llvm.unreachable` operation. @@ -152,6 +159,7 @@ pub fn bitcast<'c>( .add_operands(&[argument]) .add_results(&[result]) .build() + .expect("valid operation") } /// Creates a `llvm.alloca` operation. @@ -167,6 +175,7 @@ pub fn alloca<'c>( .add_attributes(&extra_options.into_attributes(context)) .add_results(&[ptr_type]) .build() + .expect("valid operation") } /// Creates a `llvm.store` operation. @@ -181,6 +190,7 @@ pub fn store<'c>( .add_operands(&[value, addr]) .add_attributes(&extra_options.into_attributes(context)) .build() + .expect("valid operation") } /// Creates a `llvm.load` operation. @@ -196,6 +206,7 @@ pub fn load<'c>( .add_attributes(&extra_options.into_attributes(context)) .add_results(&[r#type]) .build() + .expect("valid operation") } /// Create a `llvm.func` operation. @@ -215,6 +226,7 @@ pub fn func<'c>( .add_attributes(attributes) .add_regions(vec![region]) .build() + .expect("valid operation") } // Creates a `llvm.return` operation. @@ -229,7 +241,7 @@ pub fn r#return<'c>( builder = builder.add_operands(&[value]); } - builder.build() + builder.build().expect("valid operation") } /// Creates a `llvm.call_intrinsic` operation. @@ -245,6 +257,7 @@ pub fn call_intrinsic<'c>( .add_attributes(&[(Identifier::new(context, "intrin"), intrin.into())]) .add_results(results) .build() + .expect("valid operation") } /// Creates a `llvm.intr.ctlz` operation. @@ -264,6 +277,7 @@ pub fn intr_ctlz<'c>( .add_operands(&[value]) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.intr.ctlz` operation. @@ -283,6 +297,7 @@ pub fn intr_cttz<'c>( .add_operands(&[value]) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.intr.ctlz` operation. @@ -296,6 +311,7 @@ pub fn intr_ctpop<'c>( .add_operands(&[value]) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.intr.bswap` operation. @@ -309,6 +325,7 @@ pub fn intr_bswap<'c>( .add_operands(&[value]) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.intr.bitreverse` operation. @@ -322,6 +339,7 @@ pub fn intr_bitreverse<'c>( .add_operands(&[value]) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.intr.abs` operation. @@ -344,6 +362,7 @@ pub fn intr_abs<'c>( .add_operands(&[value]) .add_results(&[result_type]) .build() + .expect("valid operation") } /// Creates a `llvm.zext` operation. @@ -357,6 +376,7 @@ pub fn zext<'c>( .add_operands(&[value]) .add_results(&[result_type]) .build() + .expect("valid operation") } #[cfg(test)] diff --git a/melior/src/dialect/memref.rs b/melior/src/dialect/memref.rs index ba45efeabc..fb0b855643 100644 --- a/melior/src/dialect/memref.rs +++ b/melior/src/dialect/memref.rs @@ -76,7 +76,10 @@ fn allocate<'c>( builder.add_attributes(&[(Identifier::new(context, "alignment"), alignment.into())]); } - builder.add_results(&[r#type.into()]).build() + builder + .add_results(&[r#type.into()]) + .build() + .expect("valid operation") } /// Create a `memref.cast` operation. @@ -90,6 +93,7 @@ pub fn cast<'c>( .add_operands(&[value]) .add_results(&[r#type.into()]) .build() + .expect("valid operation") } /// Create a `memref.dealloc` operation. @@ -101,6 +105,7 @@ pub fn dealloc<'c>( OperationBuilder::new(context, "memref.dealloc", location) .add_operands(&[value]) .build() + .expect("valid operation") } /// Create a `memref.dim` operation. @@ -114,6 +119,7 @@ pub fn dim<'c>( .add_operands(&[value, index]) .enable_result_type_inference() .build() + .expect("valid operation") } /// Create a `memref.get_global` operation. @@ -130,6 +136,7 @@ pub fn get_global<'c>( )]) .add_results(&[r#type.into()]) .build() + .expect("valid operation") } /// Create a `memref.global` operation. @@ -178,7 +185,7 @@ pub fn global<'c>( builder.add_attributes(&[(Identifier::new(context, "alignment"), alignment.into())]); } - builder.build() + builder.build().expect("valid operation") } /// Create a `memref.load` operation. @@ -193,6 +200,7 @@ pub fn load<'c>( .add_operands(indices) .enable_result_type_inference() .build() + .expect("valid operation") } /// Create a `memref.rank` operation. @@ -205,6 +213,7 @@ pub fn rank<'c>( .add_operands(&[value]) .enable_result_type_inference() .build() + .expect("valid operation") } /// Create a `memref.store` operation. @@ -219,6 +228,7 @@ pub fn store<'c>( .add_operands(&[value, memref]) .add_operands(indices) .build() + .expect("valid operation") } /// Create a `memref.realloc` operation. @@ -243,7 +253,7 @@ pub fn realloc<'c>( builder.add_attributes(&[(Identifier::new(context, "alignment"), alignment.into())]); } - builder.build() + builder.build().expect("valid operation") } #[cfg(test)] diff --git a/melior/src/ir/block.rs b/melior/src/ir/block.rs index 74b60a87bd..575de70921 100644 --- a/melior/src/ir/block.rs +++ b/melior/src/ir/block.rs @@ -404,7 +404,9 @@ mod tests { let block = Block::new(&[]); let operation = block.append_operation( - OperationBuilder::new(&context, "func.return", Location::unknown(&context)).build(), + OperationBuilder::new(&context, "func.return", Location::unknown(&context)) + .build() + .unwrap(), ); assert_eq!(block.terminator(), Some(operation)); @@ -422,7 +424,9 @@ mod tests { let block = Block::new(&[]); let operation = block.append_operation( - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(), + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(), ); assert_eq!(block.first_operation(), Some(operation)); @@ -442,7 +446,9 @@ mod tests { let block = Block::new(&[]); block.append_operation( - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(), + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(), ); } @@ -454,7 +460,9 @@ mod tests { block.insert_operation( 0, - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(), + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(), ); } @@ -465,11 +473,15 @@ mod tests { let block = Block::new(&[]); let first_operation = block.append_operation( - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(), + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(), ); let second_operation = block.insert_operation_after( first_operation, - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(), + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(), ); assert_eq!(block.first_operation(), Some(first_operation)); @@ -486,11 +498,15 @@ mod tests { let block = Block::new(&[]); let second_operation = block.append_operation( - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(), + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(), ); let first_operation = block.insert_operation_before( second_operation, - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(), + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(), ); assert_eq!(block.first_operation(), Some(first_operation)); diff --git a/melior/src/ir/module.rs b/melior/src/ir/module.rs index 87053a02a4..9c20329002 100644 --- a/melior/src/ir/module.rs +++ b/melior/src/ir/module.rs @@ -128,7 +128,8 @@ mod tests { let module = Module::from_operation( OperationBuilder::new(&context, "builtin.module", Location::unknown(&context)) .add_regions(vec![region]) - .build(), + .build() + .unwrap(), ) .unwrap(); @@ -141,7 +142,9 @@ mod tests { let context = create_test_context(); assert!(Module::from_operation( - OperationBuilder::new(&context, "func.func", Location::unknown(&context),).build() + OperationBuilder::new(&context, "func.func", Location::unknown(&context),) + .build() + .unwrap() ) .is_none()); } diff --git a/melior/src/ir/operation.rs b/melior/src/ir/operation.rs index e3efd32a26..4380f71330 100644 --- a/melior/src/ir/operation.rs +++ b/melior/src/ir/operation.rs @@ -447,6 +447,7 @@ mod tests { assert_eq!( OperationBuilder::new(&context, "foo", Location::unknown(&context),) .build() + .unwrap() .name(), Identifier::new(&context, "foo") ); @@ -458,7 +459,9 @@ mod tests { context.set_allow_unregistered_dialects(true); let block = Block::new(&[]); let operation = block.append_operation( - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(), + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(), ); assert_eq!(operation.block().as_deref(), Some(&block)); @@ -471,6 +474,7 @@ mod tests { assert_eq!( OperationBuilder::new(&context, "foo", Location::unknown(&context)) .build() + .unwrap() .block(), None ); @@ -483,6 +487,7 @@ mod tests { assert_eq!( OperationBuilder::new(&context, "foo", Location::unknown(&context)) .build() + .unwrap() .result(0) .unwrap_err(), Error::PositionOutOfBounds { @@ -500,6 +505,7 @@ mod tests { assert_eq!( OperationBuilder::new(&context, "foo", Location::unknown(&context),) .build() + .unwrap() .region(0), Err(Error::PositionOutOfBounds { name: "region", @@ -522,7 +528,8 @@ mod tests { let operands = vec![argument, argument, argument]; let operation = OperationBuilder::new(&context, "foo", Location::unknown(&context)) .add_operands(&operands) - .build(); + .build() + .unwrap(); assert_eq!( operation.operands().skip(1).collect::>(), @@ -537,7 +544,8 @@ mod tests { let operation = OperationBuilder::new(&context, "foo", Location::unknown(&context)) .add_regions(vec![Region::new()]) - .build(); + .build() + .unwrap(); assert_eq!( operation.regions().collect::>(), @@ -555,7 +563,8 @@ mod tests { Identifier::new(&context, "foo"), StringAttribute::new(&context, "bar").into(), )]) - .build(); + .build() + .unwrap(); assert!(operation.has_attribute(&context, "foo")); assert_eq!( operation.attribute(&context, "foo").map(|a| a.to_string()), @@ -585,7 +594,9 @@ mod tests { fn clone() { let context = create_test_context(); context.set_allow_unregistered_dialects(true); - let operation = OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(); + let operation = OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(); let _ = operation.clone(); } @@ -598,6 +609,7 @@ mod tests { assert_eq!( OperationBuilder::new(&context, "foo", Location::unknown(&context),) .build() + .unwrap() .to_string(), "\"foo\"() : () -> ()\n" ); @@ -611,7 +623,9 @@ mod tests { assert_eq!( format!( "{:?}", - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build() + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap() ), "Operation(\n\"foo\"() : () -> ()\n)" ); @@ -625,6 +639,7 @@ mod tests { assert_eq!( OperationBuilder::new(&context, "foo", Location::unknown(&context)) .build() + .unwrap() .to_string_with_flags( OperationPrintingFlags::new() .elide_large_elements_attributes(100) diff --git a/melior/src/ir/operation/result.rs b/melior/src/ir/operation/result.rs index cdcf4db63a..9ae31444ad 100644 --- a/melior/src/ir/operation/result.rs +++ b/melior/src/ir/operation/result.rs @@ -73,7 +73,8 @@ mod tests { let r#type = Type::parse(&context, "index").unwrap(); let operation = OperationBuilder::new(&context, "foo", Location::unknown(&context)) .add_results(&[r#type]) - .build(); + .build() + .unwrap(); assert_eq!(operation.result(0).unwrap().result_number(), 0); } diff --git a/melior/src/ir/value.rs b/melior/src/ir/value.rs index 073e987747..276df137b7 100644 --- a/melior/src/ir/value.rs +++ b/melior/src/ir/value.rs @@ -96,7 +96,8 @@ mod tests { Identifier::new(&context, "value"), Attribute::parse(&context, "0 : index").unwrap(), )]) - .build(); + .build() + .unwrap(); assert_eq!(operation.result(0).unwrap().r#type(), index_type); } @@ -113,7 +114,8 @@ mod tests { Identifier::new(&context, "value"), Attribute::parse(&context, "0 : index").unwrap(), )]) - .build(); + .build() + .unwrap(); assert!(operation.result(0).unwrap().is_operation_result()); } @@ -139,7 +141,8 @@ mod tests { Identifier::new(&context, "value"), Attribute::parse(&context, "0 : index").unwrap(), )]) - .build(); + .build() + .unwrap(); value.result(0).unwrap().dump(); } @@ -156,7 +159,8 @@ mod tests { Identifier::new(&context, "value"), Attribute::parse(&context, "0 : index").unwrap(), )]) - .build(); + .build() + .unwrap(); let result = Value::from(operation.result(0).unwrap()); assert_eq!(result, result); @@ -176,6 +180,7 @@ mod tests { Attribute::parse(&context, "0 : index").unwrap(), )]) .build() + .unwrap() }; assert_ne!( @@ -198,7 +203,8 @@ mod tests { Identifier::new(&context, "value"), Attribute::parse(&context, "0 : index").unwrap(), )]) - .build(); + .build() + .unwrap(); assert_eq!( operation.result(0).unwrap().to_string(), @@ -219,7 +225,8 @@ mod tests { Identifier::new(&context, "value"), Attribute::parse(&context, "0 : index").unwrap(), )]) - .build(); + .build() + .unwrap(); assert_eq!( operation.result(0).unwrap().to_string(), @@ -240,7 +247,8 @@ mod tests { Identifier::new(&context, "value"), Attribute::parse(&context, "0 : index").unwrap(), )]) - .build(); + .build() + .unwrap(); assert_eq!( format!("{:?}", Value::from(operation.result(0).unwrap())), From 31e937d8a8d2e7e1b070acce7cb571c3a8742766 Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:57:16 +1100 Subject: [PATCH 10/14] Fix --- macro/src/operation.rs | 3 +++ melior/src/dialect/index.rs | 2 ++ melior/src/dialect/llvm.rs | 4 +++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/macro/src/operation.rs b/macro/src/operation.rs index f071c6b834..986fbf19f7 100644 --- a/macro/src/operation.rs +++ b/macro/src/operation.rs @@ -35,6 +35,7 @@ pub fn generate_binary(dialect: &Ident, names: &[Ident]) -> Result Result( .add_attributes(&[(Identifier::new(context, "value"), value.into())]) .enable_result_type_inference() .build() + .expect("valid operation") } /// Creates an `index.cmp` operation. @@ -54,6 +55,7 @@ pub fn cmp<'c>( .add_operands(&[lhs, rhs]) .enable_result_type_inference() .build() + .expect("valid operation") } melior_macro::binary_operations!( diff --git a/melior/src/dialect/llvm.rs b/melior/src/dialect/llvm.rs index 36b8915437..c87071423a 100644 --- a/melior/src/dialect/llvm.rs +++ b/melior/src/dialect/llvm.rs @@ -145,7 +145,9 @@ pub fn nullptr<'c>( /// Creates a `llvm.unreachable` operation. pub fn unreachable<'c>(context: &'c Context, location: Location<'c>) -> Operation<'c> { - OperationBuilder::new(context, "llvm.unreachable", location).build() + OperationBuilder::new(context, "llvm.unreachable", location) + .build() + .expect("valid operation") } /// Creates a `llvm.bitcast` operation. From e4401ab17e8e5c5991bb3d1349f68d9aff20ef5f Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:58:41 +1100 Subject: [PATCH 11/14] Fix --- melior/src/dialect/cf.rs | 6 +++++- melior/src/dialect/func.rs | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/melior/src/dialect/cf.rs b/melior/src/dialect/cf.rs index 64505d28b2..fd5db85807 100644 --- a/melior/src/dialect/cf.rs +++ b/melior/src/dialect/cf.rs @@ -26,6 +26,7 @@ pub fn assert<'c>( )]) .add_operands(&[argument]) .build() + .expect("valid operation") } /// Creates a `cf.br` operation. @@ -39,6 +40,7 @@ pub fn br<'c>( .add_operands(destination_operands) .add_successors(&[successor]) .build() + .expect("valid operation") } /// Creates a `cf.cond_br` operation. @@ -73,6 +75,7 @@ pub fn cond_br<'c>( ) .add_successors(&[true_successor, false_successor]) .build() + .expect("valid operation") } /// Creates a `cf.switch` operation. @@ -137,7 +140,8 @@ pub fn switch<'c>( .collect::>(), ) .add_successors(&destinations) - .build()) + .build() + .expect("valid operation")) } #[cfg(test)] diff --git a/melior/src/dialect/func.rs b/melior/src/dialect/func.rs index fbb036b1db..b9e5fafef0 100644 --- a/melior/src/dialect/func.rs +++ b/melior/src/dialect/func.rs @@ -23,6 +23,7 @@ pub fn call<'c>( .add_operands(arguments) .add_results(result_types) .build() + .expect("valid operation") } /// Create a `func.call_indirect` operation. @@ -38,6 +39,7 @@ pub fn call_indirect<'c>( .add_operands(arguments) .add_results(result_types) .build() + .expect("valid operation") } /// Create a `func.constant` operation. @@ -51,6 +53,7 @@ pub fn constant<'c>( .add_attributes(&[(Identifier::new(context, "value"), function.into())]) .add_results(&[r#type.into()]) .build() + .expect("valid operation") } /// Create a `func.func` operation. @@ -70,6 +73,7 @@ pub fn func<'c>( .add_attributes(attributes) .add_regions(vec![region]) .build() + .expect("valid operation") } /// Create a `func.return` operation. @@ -81,6 +85,7 @@ pub fn r#return<'c>( OperationBuilder::new(context, "func.return", location) .add_operands(operands) .build() + .expect("valid operation") } #[cfg(test)] From 2f100da20ec084fe78ae387c6c3e9d2e9b79fa49 Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 11:59:13 +1100 Subject: [PATCH 12/14] Fix --- melior/src/dialect/arith.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/melior/src/dialect/arith.rs b/melior/src/dialect/arith.rs index f3de7ab665..f0b5f27a3f 100644 --- a/melior/src/dialect/arith.rs +++ b/melior/src/dialect/arith.rs @@ -20,6 +20,7 @@ pub fn constant<'c>( .add_attributes(&[(Identifier::new(context, "value"), value)]) .enable_result_type_inference() .build() + .expect("valid operation") } /// `arith.cmpf` predicate @@ -94,6 +95,7 @@ fn cmp<'c>( .add_operands(&[lhs, rhs]) .enable_result_type_inference() .build() + .expect("valid operation") } /// Creates an `arith.select` operation. @@ -108,6 +110,7 @@ pub fn select<'c>( .add_operands(&[condition, true_value, false_value]) .add_results(&[true_value.r#type()]) .build() + .expect("valid operation") } melior_macro::binary_operations!( From ecb6edce09c226a9013124b6e8e38783808444fa Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 12:11:15 +1100 Subject: [PATCH 13/14] Fix --- melior/src/error.rs | 4 ++++ melior/src/ir/operation/builder.rs | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/melior/src/error.rs b/melior/src/error.rs index 543fdf21b8..ab5caee550 100644 --- a/melior/src/error.rs +++ b/melior/src/error.rs @@ -16,6 +16,7 @@ pub enum Error { value: String, }, InvokeFunction, + OperationBuild, OperandNotFound(&'static str), OperationResultExpected(String), PositionOutOfBounds { @@ -47,6 +48,9 @@ impl Display for Error { write!(formatter, "element of {type} type expected: {value}") } Self::InvokeFunction => write!(formatter, "failed to invoke JIT-compiled function"), + Self::OperationBuild => { + write!(formatter, "operation build failed") + } Self::OperandNotFound(name) => { write!(formatter, "operand {name} not found") } diff --git a/melior/src/ir/operation/builder.rs b/melior/src/ir/operation/builder.rs index 64fdfc2af8..fd297d9a42 100644 --- a/melior/src/ir/operation/builder.rs +++ b/melior/src/ir/operation/builder.rs @@ -3,6 +3,7 @@ use crate::{ context::Context, ir::{Attribute, AttributeLike, Block, Identifier, Location, Region, Type, Value}, string_ref::StringRef, + Error, }; use mlir_sys::{ mlirNamedAttributeGet, mlirOperationCreate, mlirOperationStateAddAttributes, @@ -119,8 +120,9 @@ impl<'c> OperationBuilder<'c> { } /// Builds an operation. - pub fn build(mut self) -> Option> { + pub fn build(mut self) -> Result, Error> { unsafe { Operation::from_option_raw(mlirOperationCreate(&mut self.raw)) } + .ok_or(Error::OperationBuild) } } From 266e98d9e5c488189ea5800ef2974a874f48d0f0 Mon Sep 17 00:00:00 2001 From: Yota Toyama Date: Wed, 18 Oct 2023 12:12:09 +1100 Subject: [PATCH 14/14] Fix --- melior/src/ir/operation.rs | 4 +++- melior/src/ir/operation/builder.rs | 19 +++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/melior/src/ir/operation.rs b/melior/src/ir/operation.rs index 4380f71330..28dc5f5929 100644 --- a/melior/src/ir/operation.rs +++ b/melior/src/ir/operation.rs @@ -436,7 +436,9 @@ mod tests { fn new() { let context = create_test_context(); context.set_allow_unregistered_dialects(true); - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(); + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(); } #[test] diff --git a/melior/src/ir/operation/builder.rs b/melior/src/ir/operation/builder.rs index fd297d9a42..aeca4614a1 100644 --- a/melior/src/ir/operation/builder.rs +++ b/melior/src/ir/operation/builder.rs @@ -139,7 +139,9 @@ mod tests { let context = create_test_context(); context.set_allow_unregistered_dialects(true); - OperationBuilder::new(&context, "foo", Location::unknown(&context)).build(); + OperationBuilder::new(&context, "foo", Location::unknown(&context)) + .build() + .unwrap(); } #[test] @@ -154,7 +156,8 @@ mod tests { OperationBuilder::new(&context, "foo", Location::unknown(&context)) .add_operands(&[argument]) - .build(); + .build() + .unwrap(); } #[test] @@ -164,7 +167,8 @@ mod tests { OperationBuilder::new(&context, "foo", Location::unknown(&context)) .add_results(&[Type::parse(&context, "i1").unwrap()]) - .build(); + .build() + .unwrap(); } #[test] @@ -174,7 +178,8 @@ mod tests { OperationBuilder::new(&context, "foo", Location::unknown(&context)) .add_regions(vec![Region::new()]) - .build(); + .build() + .unwrap(); } #[test] @@ -184,7 +189,8 @@ mod tests { OperationBuilder::new(&context, "foo", Location::unknown(&context)) .add_successors(&[&Block::new(&[])]) - .build(); + .build() + .unwrap(); } #[test] @@ -197,7 +203,8 @@ mod tests { Identifier::new(&context, "foo"), Attribute::parse(&context, "unit").unwrap(), )]) - .build(); + .build() + .unwrap(); } #[test]