AIE Python Bindings #716

AndraBisca · 2023-11-03T16:11:18Z

This PR introduces an extension of the auto-generated python bindings we currently have in the repo. In particular, the extended operations have specialised constructors that can take in python integers as inputs. For now, only the most widely used operations in the AIE dialect have been extended. The others will be added in future PRs.

This PR relies on #715

makslevental

In general I think this isn't the right thing to do because

There are now value builders being generated automatically [mlir][python] generate value builders llvm/llvm-project#68308 (if you check in _AIE_ops_gen.py you will see them just under the conventional builders);
Shortly they will support infix operators [mlir][python] value casting llvm/llvm-project#69644;
My next upstream PR will be region_op.

makslevental · 2023-11-03T18:13:07Z

python/dialects/_AIE_syntax.py

+
+
+# Create an aie device on specified target architecture.
+class Device(DeviceOp):


This is going to break as soon as we bump due to this PR I landed upstream llvm/llvm-project#68853.

Hey Maks, thank you for your detailed review! I looked through the PRs you mentioned above. Could you please help me understand why this would break exactly? In the PR above it looks like extensions that were calling super.init() can stay unchanged.

You're missing the registration decorators register_operation(_Dialect, replace=True) (like here). Also btw with this particular builder, you're better off just registering an attribute builder for AIEDevice (same with all of these that are basically making attributes in the super) or even better changing AIEDevice to be an I32EnumAttr so that its attribute builder is free (see here).

Sorry I didn't realize that AIEDevice is already an EnumAttr. So you can get this for free by signing up using GEN_ENUM_BINDINGS.

python/dialects/_AIE_util.py

fifield · 2023-11-03T20:00:58Z

In general I think this isn't the right thing to do because

There are now value builders being generated automatically [mlir][python] generate value builders llvm/llvm-project#68308 (if you check in _AIE_ops_gen.py you will see them just under the conventional builders);

Shortly they will support infix operators [mlir][python] value casting llvm/llvm-project#69644;

My next upstream PR will be region_op.

We have a set of examples and tests using these bindings and we are maintaining them downstream. This is one in a set of PRs to get all that landed. Are these all things we can apply incrementally to what we have?

makslevental · 2023-11-03T20:33:48Z

Are these all things we can apply incrementally to what we have?

You can just choose not to use it but it's all exactly the same functionality (and more). My only point is why have code you have to maintain when upstream is already maintaining the same code with the same functionality.

makslevental · 2023-11-03T20:54:18Z

@fifield where are these examples? Can you link so I can see how they're written?

jgmelber · 2023-11-03T20:56:44Z

@makslevental this is representative of the style in the examples: test/python/codeRegion.py

makslevental · 2023-11-03T21:04:54Z

@makslevental this is representative of the style in the examples: test/python/codeRegion.py

Plenty that can be "regularized" there. For example, you can already rewrite this to use just regular python for loops https://github.com/llvm/llvm-project/blob/main/mlir/test/python/dialects/scf.py#L64.

AndraBisca · 2023-11-06T16:41:59Z

I understand that what I have hand-implemented in this PR will become auto-generated in the near future. I look forward to that and will be ready to modify all of these examples accordingly. However, because of internal deadlines, our priority right now is to support all the downstream work that is currently using these simple sugar bindings. We should discuss internally how this aligns with the release of your automated python bindings, @makslevental

makslevental · 2023-11-06T16:58:00Z

I understand that what I have hand-implemented in this PR will become auto-generated in the near future. I look forward to that and will be ready to modify all of these examples accordingly. However, because of internal deadlines, our priority right now is to support all the downstream work that is currently using these simple sugar bindings. We should discuss internally how this aligns with the release of your automated python bindings, @makslevental

Whoops finget slipped...

Anyway of course that's reasonable. I was just trying to emphasize that some of those PRs are only a couple of days away from being merged. Anyway I'll just port this code here when they're merged.

AndraBisca · 2023-11-06T17:05:10Z

I understand that what I have hand-implemented in this PR will become auto-generated in the near future. I look forward to that and will be ready to modify all of these examples accordingly. However, because of internal deadlines, our priority right now is to support all the downstream work that is currently using these simple sugar bindings. We should discuss internally how this aligns with the release of your automated python bindings, @makslevental

Whoops finget slipped...

Anyway of course that's reasonable. I was just trying to emphasize that some of those PRs are only a couple of days away from being merged. Anyway I'll just port this code here when they're merged.

Sounds good! I will still inquire more about the deadlines, because we might be able to wait(?). I'll let you know!

stephenneuendorffer · 2023-11-07T04:00:24Z

Sounds good! I will still inquire more about the deadlines, because we might be able to wait(?). I'll let you know!

I think we really need to move to using the bindings that are upstream. There's no real value in diverging from that, as far as I can tell. I think in the short term, we need to make sure we can get up to HEAD and then propose a time frame to migrate the python bindings. @AndraBisca can you figure out a time frame for that?

makslevental · 2023-11-07T04:40:16Z

we need to make sure we can get up to HEAD

my contribution: #724 :)

AndraBisca · 2023-11-07T16:41:45Z

Sounds good! I will still inquire more about the deadlines, because we might be able to wait(?). I'll let you know!

I think we really need to move to using the bindings that are upstream. There's no real value in diverging from that, as far as I can tell. I think in the short term, we need to make sure we can get up to HEAD and then propose a time frame to migrate the python bindings. @AndraBisca can you figure out a time frame for that?

Yes, I can do that!

makslevental · 2023-11-07T17:08:53Z

This upstream PR has been merged: llvm/llvm-project#69644. I have been working on this feature for ~9 months 🎉.

What this means is we can now transition to using infix operators for all operations (that we choose to implement for).

The way this works is described in the PR but I'll recapitulate here: the only thing ¹ we need to do is register a "value caster":

@register_value_caster(F16Type.static_typeid)
@register_value_caster(F32Type.static_typeid)
@register_value_caster(F64Type.static_typeid)
@register_value_caster(IntegerType.static_typeid)
class ArithValue(Value):
    __add__ = partialmethod(_binary_op, op="add")
    __sub__ = partialmethod(_binary_op, op="sub")
    __mul__ = partialmethod(_binary_op, op="mul")

and upstream will take care of the rest:

a = arith.constant(value=FloatAttr.get(f16_t, 42.42))
b = a + a
# CHECK: ArithValue(%0 = arith.addf %cst, %cst : f16)
print(b)

a = arith.constant(value=FloatAttr.get(f32_t, 42.42))
b = a - a
# CHECK: ArithValue(%1 = arith.subf %cst_0, %cst_0 : f32)
print(b)

a = arith.constant(value=FloatAttr.get(f64_t, 42.42))
b = a * a
# CHECK: ArithValue(%2 = arith.mulf %cst_1, %cst_1 : f64)
print(b)

I'll plan to start rolling this out soon (this week) but it should be easy enough for anyone to jump in.

And transition to using value builders instead of operation builders. ↩

…sts.

jgmelber

LGTM

AndraBisca requested review from fifield and jgmelber November 3, 2023 16:20

fifield approved these changes Nov 3, 2023

View reviewed changes

makslevental reviewed Nov 3, 2023

View reviewed changes

jgmelber approved these changes Nov 3, 2023

View reviewed changes

makslevental closed this Nov 6, 2023

makslevental reopened this Nov 6, 2023

makslevental self-requested a review November 6, 2023 16:58

makslevental approved these changes Nov 6, 2023

View reviewed changes

makslevental force-pushed the python-bindings branch from ade2e94 to f3b7943 Compare November 7, 2023 03:23

AndraBisca marked this pull request as draft November 8, 2023 17:10

abisca and others added 4 commits November 13, 2023 14:23

Add simple test using MLIR generated python bindings.

389723d

Add AIE dialect python bindings as well as some util bindings. Add te…

cbfe335

…sts.

use AIEDevice from enum_gen

4944337

use WireBundle and ObjectFifoPort

c10abfd

makslevental force-pushed the python-bindings branch from 55a80b9 to c10abfd Compare November 13, 2023 20:37

makslevental added 2 commits November 13, 2023 14:47

fix tests

ea1cef1

black/format

d390876

add ipu examples

ff0a808

makslevental force-pushed the python-bindings branch from 0d889ad to ff0a808 Compare November 13, 2023 21:50

makslevental added 2 commits November 13, 2023 16:00

failing test

fe05e59

passing test

201a4e5

jgmelber self-requested a review November 13, 2023 22:12

jgmelber approved these changes Nov 13, 2023

View reviewed changes

jgmelber requested review from denolf and jackl-xilinx November 13, 2023 22:26

makslevental marked this pull request as ready for review November 13, 2023 22:33

makslevental merged commit cd86e0d into main Nov 13, 2023
6 checks passed

makslevental deleted the python-bindings branch November 13, 2023 23:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AIE Python Bindings #716

AIE Python Bindings #716

AndraBisca commented Nov 3, 2023

makslevental left a comment •

edited

Loading

makslevental Nov 3, 2023

AndraBisca Nov 6, 2023

makslevental Nov 6, 2023 •

edited

Loading

makslevental Nov 7, 2023 •

edited

Loading

fifield commented Nov 3, 2023

makslevental commented Nov 3, 2023 •

edited

Loading

makslevental commented Nov 3, 2023

jgmelber commented Nov 3, 2023 •

edited

Loading

makslevental commented Nov 3, 2023 •

edited

Loading

AndraBisca commented Nov 6, 2023 •

edited

Loading

makslevental commented Nov 6, 2023

AndraBisca commented Nov 6, 2023

stephenneuendorffer commented Nov 7, 2023

makslevental commented Nov 7, 2023 •

edited

Loading

AndraBisca commented Nov 7, 2023

makslevental commented Nov 7, 2023 •

edited

Loading

jgmelber left a comment



		# Create an aie device on specified target architecture.
		class Device(DeviceOp):

AIE Python Bindings #716

AIE Python Bindings #716

Conversation

AndraBisca commented Nov 3, 2023

makslevental left a comment • edited Loading

Choose a reason for hiding this comment

makslevental Nov 3, 2023

Choose a reason for hiding this comment

AndraBisca Nov 6, 2023

Choose a reason for hiding this comment

makslevental Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

makslevental Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

fifield commented Nov 3, 2023

makslevental commented Nov 3, 2023 • edited Loading

makslevental commented Nov 3, 2023

jgmelber commented Nov 3, 2023 • edited Loading

makslevental commented Nov 3, 2023 • edited Loading

AndraBisca commented Nov 6, 2023 • edited Loading

makslevental commented Nov 6, 2023

AndraBisca commented Nov 6, 2023

stephenneuendorffer commented Nov 7, 2023

makslevental commented Nov 7, 2023 • edited Loading

AndraBisca commented Nov 7, 2023

makslevental commented Nov 7, 2023 • edited Loading

Footnotes

jgmelber left a comment

Choose a reason for hiding this comment

makslevental left a comment •

edited

Loading

makslevental Nov 6, 2023 •

edited

Loading

makslevental Nov 7, 2023 •

edited

Loading

makslevental commented Nov 3, 2023 •

edited

Loading

jgmelber commented Nov 3, 2023 •

edited

Loading

makslevental commented Nov 3, 2023 •

edited

Loading

AndraBisca commented Nov 6, 2023 •

edited

Loading

makslevental commented Nov 7, 2023 •

edited

Loading

makslevental commented Nov 7, 2023 •

edited

Loading