rebase

add import test

amazing

Yep thanks for the suggestion

IMO that would make sense to me. Anything to land these changes incrementally.

In D144693#4172706, @math-fehr wrote:

In D144693#4172489, @rriddle wrote:

This patch seems to be doing a lot of things at once. I would split out registration in mlir-opt from the various IRDL things being added, and make things a bit more incremental, given the IRDL constructs could be tested via a test-pass (for example).

Do you mean removing the MLIROptMain change for another patch, and then test the rest of the patch in a test-pass (I guess I can add features one by one, like first registration of types/attributes, then of operations).

If I'm correct, you are proposing to have a pass similar to the PDL one that register rewrites that are part of the file (with the -test-pdl-bytecode-pass pass). However, I couldn't do something similar with IRDL, since we cannot parse the entire file until the attributes are registered from what I understand.

Is that what you meant? I probably misunderstood something

LGTM

LGTM but River mentioned there were some perf issues. Do have a quick capture of the before vs. after time to run a pattern-heavy workload?

mostly stylistic comments

+1 to what Alex said, but otherwise LGT

+1 finally I'm happy to see this

FYI there's still mention of this folder API thing in the MLIR docs

please add a test

comments

This change overall makes sense to me

Is this meant to fix something?

Please add a test

sure

LGTM. mlir-translate is intended first and foremost to be a testing tool. Making it easier to test with makes sense to me.

landed

Was there a discourse discussion on this? I might've missed it. If there isn't, I'm 100% behind the rationale I think is behind this change, but I would still like to see it discussed.

I think a unit test would've been more appropriate in this case, but LGTM

I assume AttrTypeHandler is now the only way to override the walk behaviour for an attribute or type? (And provide it for unusual attributes and types that might be written in C++..)

nice

LGTM, thanks!

I don't understand how this fixes the issue. How are you able to dereference an uninitialized optional?

nuceu

rebase+review comments

rebase

nice

review comments and update description with migration instructions

There's a build option to disable TableGen warnings. That should work, of course.

fix build

review comments

thank

Yup

IRMapping

rebase