mlir-rs/melior

API for ArrayAttr

Closed this issue · 0 comments

phase commented

I see there's a rustic API for DenseI64ArrayAttribute & DenseI32ArrayAttribute, but I don't see one for a normal ArrayAttribute. There are a bunch of functions in the vector dialect that use attributes with the ::mlir::ArrayAttr type, and using a dense array here will result in an error.

Rust to build the op:

            let vector_extract_op = OperationBuilder::new("vector.extract", location)
                .add_attributes(&[(
                    Identifier::new(&context, "position"),
                    DenseI64ArrayAttribute::new(&context, &[0]).into()
                )])
                .add_operands(&[block.argument(0).unwrap().into()])
                .add_results(&[float32_type])
                .build();

Resulting MLIR for this op:

%0 = "vector.extract"(%arg0) {position = array<i64: 0>} : (vector<2xf32>) -> f32
    "func.return"(%0) : (f32) -> ()

error:

error: 'vector.extract' op attribute 'position' failed to satisfy constraint: 64-bit integer array attribute
thread 'main' panicked at 'assertion failed: module_op.verify()', src/main.rs:79:5

Also, while playing with melior I've noticed a lot of structs / functions are marked private or pub(crate). I think it may be more ergonomic to mark these pub & unsafe to let users get the raw MLIR handles while the safe API is still being worked on. For example, I believe this should work:

mlir_sys::mlirArrayAttrGet(context.to_raw(), 1, &IntegerAttribute::new(1, index_type).to_raw());

IntegerAttribute::to_raw() is safe & public, but Context::to_raw() is pub(crate) unsafe!