tool: tblgen-to-py script #3210

alexarice · 2024-09-23T16:24:29Z

This is the start of a "tblgen-to-py" script for reading the json output of llvm-tblgen --dump-json and creating an xdsl dialect. I would like some help/advice on how to best integrate this into the existing tooling. I think something similar to the "irdl-to-pyrdl" script would be good, where the irdl file is used to create a python file.

I've included some json files for reference, but as they are huge I expect we would want to cache the result of this script in practice.

Quite a few constraints are missing at the moment but this can be iterated on.

codecov · 2024-09-23T16:29:50Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@cd2d585). Learn more about missing BASE report.
Report is 37 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3210   +/-   ##
=======================================
  Coverage        ?   89.91%           
=======================================
  Files           ?      440           
  Lines           ?    55279           
  Branches        ?     8624           
=======================================
  Hits            ?    49705           
  Misses          ?     4149           
  Partials        ?     1425

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

alexarice · 2024-09-24T13:37:51Z

Would it likely be better to go with "script that prints out some python code" than what I've done here? Programatically generating the classes seems to introduce some difficulties and makes caching harder

tobiasgrosser · 2024-09-24T13:39:56Z

Would it likely be better to go with "script that prints out some python code" than what I've done here? Programatically generating the classes seems to introduce some difficulties and makes caching harder

I think that would make a lot of sense.

alexarice · 2024-09-25T17:42:30Z

Just pushed version 2 which just creates a python file. Think this is close to being ready, just need to work out how to test it

alexarice · 2024-09-26T08:11:18Z

Struggling to come up with a way to test this, as I'd ideally like a filecheck test similar to the one for irdl-to-pyrdl. The optional I can think of are:

Include a 17000 line json in the repository (surely not ok)
Manually cull test.json to the parts that are actually relevant
Make an automated way to cull the tblgen json files
Dynamically generate a test.json (needs access to llvm-tblgen and the mlir include files). This has the advantage of testing stuff a bit more end-to-end but I can't think of a good way to ensure access to the include files

Would love to hear any good ideas for this.

tobiasgrosser · 2024-09-26T08:49:59Z

Manually cull test.json to the parts that are actually relevant

Either this or a manually crafted json file that tests the key features?

AntonLydike

Couldn't get through all the code yet, but can you add some more comments, and maybe a readme for this tool? Oh, also some minimal tests would be super neat to see what it actually does!

index.json

test.json

AntonLydike · 2024-09-26T12:42:06Z

xdsl/tools/tblgen_to_py.py

+
+@dataclass
+class TblgenLoader:
+    js: Any


Can you add a docstring to the class (and this attribute) to explain roughly what it does?

AntonLydike · 2024-09-26T12:45:28Z

@alexarice I think hand-crafting a json example would a good way to test this

alexarice · 2024-09-30T09:55:02Z

This is quite a dump of code, no? Is there no way to make this smaller/split the changes up?

I could remove all the argument constraints for a first pass, which would simplify it a little

Is there a reason why you go directly from TableGen, instead of doing the TableGen -> IRDL -> Python route? My guess is that IRDL cannot express what you currently want?

I agree with the end goal. At least short term my problems with it are:

People aren't writing dialects in irdl, and probably won't be soon.
There are a lot of constraints that can't be represented nicely in irdl at the moment (e.g. things like the shaped types)
To add a new feature to IRDL, you need to update IRDL in mlir, update the tblgen-to-irdl script, and update the xdsl irdl importer. In contrast this only relies on the json tablegen dump which shouldn't need modification.
It is still unclear to me how to nicely interface with builtin types in irdl. For example, in mlir the builtin Complex type is not parametrized. In this tool we can ignore the mlir representation and just use the xdsl one which is parametrized.

math-fehr · 2024-09-30T18:27:36Z

This is quite a dump of code, no? Is there no way to make this smaller/split the changes up?

I could remove all the argument constraints for a first pass, which would simplify it a little

Is there a reason why you go directly from TableGen, instead of doing the TableGen -> IRDL -> Python route? My guess is that IRDL cannot express what you currently want?

I agree with the end goal. At least short term my problems with it are:
1. People aren't writing dialects in irdl, and probably won't be soon.

2. There are a lot of constraints that can't be represented nicely in irdl at the moment (e.g. things like the shaped types)

3. To add a new feature to IRDL, you need to update IRDL in mlir, update the tblgen-to-irdl script, and update the xdsl irdl importer. In contrast this only relies on the json tablegen dump which shouldn't need modification.

4. It is still unclear to me how to nicely interface with builtin types in irdl. For example, in mlir the builtin `Complex` type is not parametrized. In this tool we can ignore the mlir representation and just use the xdsl one which _is_ parametrized.

Yeah I agree, let's do this right now, we'll see later if IRDL turns out to be usable enough to make it pass through it ;)

alexarice · 2024-10-02T09:20:46Z

Think I've fixed up all the comments, was there a consensus on trying to split this up into separate PRs?

xdsl/tools/tblgen_to_py.py

tests/tblgen_to_py/test.py

superlopuh

The actual dialect definition is missing, do we want to include it? Also, the dialect name is missing from all the ops.

It would be great to have more documentation about this, like how the json was generated

superlopuh · 2024-10-02T14:55:03Z

tests/tblgen_to_py/test_tblgen.py

+    output = subprocess.run(
+        [
+            "xdsl-tblgen",
+            "-i",
+            "tests/tblgen_to_py/test.json",
+        ],
+        capture_output=True,
+        text=True,
+    )
+
+    out_str = output.stdout


Do we need to go via subprocess here now that we've separated the command-line parsing from the generation? This feels more like an integration test than a unit test to me...

superlopuh · 2024-10-02T14:57:06Z

xdsl/tools/tblgen_to_py.py

+    if output_file is not None:
+        with open(output_file, "w") as out_file:
+            print(json.dumps(culled), file=out_file)
+    else:
+        print(json.dumps(culled))


it feels like the caller should do this dance, and the out parameter here should be IO[str] | None

superlopuh · 2024-10-02T15:01:50Z

tests/tblgen_to_py/test.py

+
+@irdl_attr_definition
+class Test_SingletonAType(ParametrizedAttribute, TypeAttribute):
+    """"""


Might as well also drop these empty docstrings

alexarice · 2024-10-02T15:35:03Z

Ready for review again

This is the start of a "tblgen-to-py" script for reading the json output of `llvm-tblgen --dump-json` and creating an xdsl dialect.

alexarice added 2 commits September 23, 2024 17:20

tblgen-to-py script

575e4bb

Add example json files

720637d

alexarice added enhancement New feature or request help wanted Extra attention is needed question Further information is requested dialects Changes on the dialects tool labels Sep 23, 2024

alexarice requested review from superlopuh, AntonLydike and math-fehr September 23, 2024 16:24

alexarice self-assigned this Sep 23, 2024

alexarice marked this pull request as draft September 23, 2024 16:24

alexarice added 2 commits September 25, 2024 18:36

v2

d795bdc

Fix UnitAttr

08598c8

alexarice added 3 commits September 25, 2024 18:45

pyright fix

960e7b7

f-expression fix

ecf1b7f

Make input and output files optional

65cefb7

AntonLydike reviewed Sep 26, 2024

View reviewed changes

alexarice added 5 commits September 27, 2024 09:21

Add docstrings

d1bc16a

Make some methods private

1e7227c

Remove index.json

86afc34

Add some type safety

0cacec3

add json culling

88385b8

alexarice added 3 commits September 30, 2024 10:58

snake case

64dc14a

Load -> import

8ebb315

Deindent

ad40426

Import -> generate

7c13be4

superlopuh reviewed Oct 2, 2024

View reviewed changes

xdsl/tools/tblgen_to_py.py Show resolved Hide resolved

superlopuh reviewed Oct 2, 2024

View reviewed changes

tests/tblgen_to_py/test.py Outdated Show resolved Hide resolved

superlopuh reviewed Oct 2, 2024

View reviewed changes

tests/tblgen_to_py/test.py Show resolved Hide resolved

superlopuh reviewed Oct 2, 2024

View reviewed changes

alexarice added 6 commits October 2, 2024 13:49

Summaries

421b1f7

Op names should include dialect

3ee32b2

Prop defs should set prop_name

19b8cde

Split main file

cd01f5f

Include dialect definition

037a9c4

Add doc

476d7a0

alexarice requested a review from superlopuh October 2, 2024 14:50

superlopuh reviewed Oct 2, 2024

View reviewed changes

alexarice added 4 commits October 2, 2024 16:16

Move output-file logic

b53e7de

Move output_file logic further out

359b851

Don't use subprocess for test

97ac90c

Remove more summaries

87f2b83

superlopuh approved these changes Oct 2, 2024

View reviewed changes

alexarice merged commit 6a3446a into main Oct 3, 2024
14 checks passed

alexarice deleted the alexarice/tblgen-to-py branch October 3, 2024 08:36

emmau678 pushed a commit that referenced this pull request Oct 8, 2024

tool: tblgen-to-py script (#3210)

36bcead

This is the start of a "tblgen-to-py" script for reading the json output of `llvm-tblgen --dump-json` and creating an xdsl dialect.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tool: tblgen-to-py script #3210

tool: tblgen-to-py script #3210

alexarice commented Sep 23, 2024

codecov bot commented Sep 23, 2024 •

edited

Loading

alexarice commented Sep 24, 2024

tobiasgrosser commented Sep 24, 2024

alexarice commented Sep 25, 2024 •

edited

Loading

alexarice commented Sep 26, 2024 •

edited

Loading

tobiasgrosser commented Sep 26, 2024

AntonLydike left a comment

AntonLydike Sep 26, 2024

AntonLydike commented Sep 26, 2024

alexarice commented Sep 30, 2024

math-fehr commented Sep 30, 2024

alexarice commented Oct 2, 2024

superlopuh left a comment

superlopuh Oct 2, 2024

superlopuh Oct 2, 2024

superlopuh Oct 2, 2024

alexarice commented Oct 2, 2024

tool: tblgen-to-py script #3210

tool: tblgen-to-py script #3210

Conversation

alexarice commented Sep 23, 2024

codecov bot commented Sep 23, 2024 • edited Loading

Codecov Report

alexarice commented Sep 24, 2024

tobiasgrosser commented Sep 24, 2024

alexarice commented Sep 25, 2024 • edited Loading

alexarice commented Sep 26, 2024 • edited Loading

tobiasgrosser commented Sep 26, 2024

AntonLydike left a comment

Choose a reason for hiding this comment

AntonLydike Sep 26, 2024

Choose a reason for hiding this comment

AntonLydike commented Sep 26, 2024

alexarice commented Sep 30, 2024

math-fehr commented Sep 30, 2024

alexarice commented Oct 2, 2024

superlopuh left a comment

Choose a reason for hiding this comment

superlopuh Oct 2, 2024

Choose a reason for hiding this comment

superlopuh Oct 2, 2024

Choose a reason for hiding this comment

superlopuh Oct 2, 2024

Choose a reason for hiding this comment

alexarice commented Oct 2, 2024

codecov bot commented Sep 23, 2024 •

edited

Loading

alexarice commented Sep 25, 2024 •

edited

Loading

alexarice commented Sep 26, 2024 •

edited

Loading