This is an archive of the discontinued LLVM Phabricator instance.

[MLIR][python bindings] Add casting of concrete types when returning from bindings code
AbandonedPublic

Authored by makslevental on May 11 2023, 9:44 PM.

Details

Summary

abandoned in favor of D150927

This diff adds casting of MlirType to an instance of PyConcreteType<> (when possible) for returns from cpp. Note, only returns. Such casting greatly improves the APIs as the user then doesn't need to cast/wrap in Python. For example VectorType(Type.parse("vector<2x3xf32>")) becomes just Type.parse("vector<2x3xf32>").

More importantly, PyValue.type now returns a concrete type (when possible); e.g.,

cst = arith.ConstantOp(F32Type.get(), 0.0).result
print(type(cst.type).__name__)
print(repr(cst.type))

prints

F32Type
F32Type(f32)

Attributes as well:

attr = DenseElementsAttr(Attribute.parse("dense<123> : tensor<i16>"))
print(repr(attr.type))
print(repr(attr.type.element_type))

prints

RankedTensorType(tensor<i16>)
IntegerType(i16)

This paves the way for writing code (both cpp and Python) that is "type conscious" (see this draft).

One current limitation is that mlir_type_subclass can't participate but I think I can make that work too (in a follow-up diff).

Diff Detail

Event Timeline

makslevental created this revision.May 11 2023, 9:44 PM
Herald added a project: Restricted Project. · View Herald Transcript

refactor switch into IRTypes.h

order correctly the macro and add tensor tests

nit re unranked type

handle element_type for containers

handle typed attr

clean up tests

add complex type and factor tests again

add tuple type

add function type

makslevental published this revision for review.May 12 2023, 1:00 PM
makslevental retitled this revision from [MLIR][python bindings] Add support for casting concrete types when returning from bindings code to [MLIR][python bindings] Add casting of concrete types when returning from bindings code.
makslevental edited the summary of this revision. (Show Details)
Herald added a project: Restricted Project. · View Herald TranscriptMay 12 2023, 1:01 PM
makslevental added inline comments.May 12 2023, 1:24 PM
mlir/test/python/ir/builtin_types.py
592–594

There's a juicy bug lurking here: if you don't insert into a region for this op (e.g., either func or module) you will get an assert fail at the end of the script:

error: 'arith.constant' op operation destroyed but still has uses
LLVM ERROR: operation destroyed but still has uses

If you dump the op after creation you get

%0 = "tensor.from_elements"(<<UNKNOWN SSA VALUE>>) : (f32) -> tensor<*xf32>

So the op is built successfully and "uses" c0 but the bindings aren't aware (I guess liveOperations is skipping a beat somehow). Note, <<UNKNOWN SSA VALUE>> is because there's no enclosing region, not because it's actually a null SSA value (I should've made that print <<UNATTACHED SSA VALUE>>...)

rewrite macro

rewrite macro

makslevental added inline comments.May 12 2023, 2:27 PM
mlir/lib/Bindings/Python/IRModule.h
1188–1206

Someone with better macro-fu might be able to suggest a prettier/smarter way to do this...

makslevental edited the summary of this revision. (Show Details)May 12 2023, 2:37 PM

rename macro

rkayaith added inline comments.May 13 2023, 9:56 AM
mlir/lib/Bindings/Python/IRModule.h
1148

I think we'd really want something that works for downstream types too. Have you looked at how the opview downcasting works? There's an op name -> python class mapping maintained, and I imagine that could be generalized to a typeid -> python class map to work with attributes and types as well.

makslevental added inline comments.May 13 2023, 10:26 AM
mlir/lib/Bindings/Python/IRModule.h
1148

that could be generalized to a typeid -> python class map

Can it? Can you stick typeids into a map? Hmmm digging around I see

/// Checks if two types are equal.
MLIR_CAPI_EXPORTED bool mlirTypeEqual(MlirType t1, MlirType t2);

which is enough for std::unordered_map (and the like) I think.

But you would need "sentinel" instances of the types to play the role of keys? Alternatively maybe you mean literally the integer? I'm pretty laissez faire so I support that but what do other people think? But these would be fully-refined right? So tensor<1x1xf32> and tensor<1x2xf32> wouldn't have the same typeid and therefore wouldn't produce a hit.

But there's another way that's possible, that I planned for the follow up, which is to jus add a case in the type_caster that queries a map that's mlir_type_subclass.__name__ -> mlir_type_subclass. Putting it before or after the builtin types (so the match happens early) can be debated.

The only issue is the case matching overhead but how bad could it be (famous last words...). This is probably totally wild but I could write a multithreaded match ("try all mlirTypeIsA##CONCRETETYPE functions and first one wins") but that's probably overengineering.

makslevental added inline comments.May 13 2023, 10:32 AM
mlir/lib/Bindings/Python/IRModule.h
1148

Yea I just double check the typeid thing won't work:

ten.type
>>> Type(tensor<10x10xi32>)
twenty.type
>>> Type(tensor<20x20xi32>)
ten.type == twenty.type
>>> False
rkayaith added inline comments.May 13 2023, 11:24 AM
mlir/lib/Bindings/Python/IRModule.h
1148

By typeid I mean this guy: https://mlir.llvm.org/doxygen/classmlir_1_1Type.html#accf7dade447f73f5b04c46fcf616b68c

I don't think any of that is exposed through the bindings yet though

makslevental added inline comments.May 13 2023, 11:55 AM
mlir/lib/Bindings/Python/IRModule.h
1148

Oh you're right - I was conflating the types themselves with typeids. Okay cool I'll send up another PR that exposes the necessary things and then adjust this one.

makslevental abandoned this revision.May 18 2023, 8:32 PM
makslevental edited the summary of this revision. (Show Details)