LG, but please add rationale in both description and comment in code.

It seems like your latest update removed the functional change (it just shows me the formatting change now)

LG to me, and optionally done via populate, I'm not sure if always good idea vs letting lower level codegen handle it (seems direct). But given not supported on all backends, SGTM.

Nice (sorry forgot to hit submit)

I see on the screenshot for unknown dialects it treats everything as unknown (e.g., memref not identified). That's more conservative (only show what you know) but might be worse for highlighting. Do you know if this is intrinsic to tree-sitter or the way the grammar is written? (It was written directly matching our markdown/uses to fix markdown, with explicit structure in mind).

Nice speedup! Sorry I missed the review notification

Fix build

Looks good thanks

Update to move common helpers to base & document defs/classes more.

I don't think you've selected reviewers yet (Nicolas seems unlikely as being selected, perhaps some overly broad Herald rule).

Remember to clean up commit message

In D145762#4184158, @mehdi_amini wrote:

~3.4 MB reduction in binary in release mode.

I don’t know how to interpret this without the total binary size, what is the percentage?

In D145250#4177310, @springerm wrote:

In D145250#4176525, @jpienaar wrote:

Should this be renamed now? E.g., reifyResultShapes now only sometimes reify. (I think folks are also reaching here for something closer to what onnx-mlir and others are doing)

Maybe, depending on what "reification" means. If you interpret it as "building some handle that you can pass around in C++" (Value or Attribute), the name is still correct.

Btw, I am preparing additional changes to this interface that would allow users to reify specific dimensions and populate a FlatAffineValueConstraints (so that the "reification results" of multiple chained ops can be combined and analyzed).

(which effectively means no not canonical, but one has sections with clear expectations and post "deployment" profile verification there should be none of these for consumers)

In D145336#4177567, @manupak wrote:

Quick question @eric-k256 @AmosLewis,

Will it also fold it to be non-F64 ? i.e. tosa consumers are able to not to witness f64 when tosa is in the canonical form.

Should this be renamed now? E.g., reifyResultShapes now only sometimes reify. (I think folks are also reaching here for something closer to what onnx-mlir and others are doing)

Fix format

Could this be done only if detected interactive session? E.g., when piping commands I'd prefer not to see this message (which is probably the majority of times when I use opt).

I was wondering (post review) if we should split the reader and writer commit parts, so that we could give a bit of time for bytecode consumers to get updated first (thinking of projects that span multiple repos). I mean, it is unstable at the moment, but wouldn't cause any additional churn.

Nice! I'm fine with delaying textual form.

Fix formatting

In D145288#4169715, @jpienaar wrote:

In D145288#4169046, @ftynse wrote:

Do we need to also update the lowering to LLVM? It may assume type is always index and end up using a different bitwidth.

I checked the final assembly output and saw different outputs as I changed width here. But let me double check the code.

In D145288#4169046, @ftynse wrote:

Do we need to also update the lowering to LLVM? It may assume type is always index and end up using a different bitwidth.

Updated post discussion to immediately use it to replace builtin dialect
reader/writer instead of starting more separate. Also bootstrapping was to
clarify unstable nature, but was called out to be too emphasized.

Thanks!

Could you add a test to show both usages/avoid regressions?

Thanks!

Is there some paper or tool or approximation method that we could cite here?

Could we just have it in its own group/approximations pass? (Well and we can populate with patterns from polynomial if needed). Just feels wrong to have this in polynomial

(started a scan but realized I wasn't really reading with any detail ...)

Nit: commit message. The rest looks fine (I forget if we allowed default values in OpBuilder ... )

When I did the first version I was using this as I wanted to run some analysis tool that uses tree-sitter (that didn't work well) and also to check the language guide a bit using the error checks in tree-sitter generation process. Generation would be really nice, and from maintenance point of view (and we know these may go out of sync, so it would be best effort). Could you perhaps show the before and after of how it affects highlighting? Esp with an unsupported dialect too, to speak to the fuzzing matching part.

Nice, thanks

I'll help in a bit

Nice, thanks

Nice, thanks. So this doesn't remove and just flips the default, so folks who haven't yet updated can just set raw explicit for now (and then fixup quickly before option is deprecated).

Sending quick fix.

Thanks!

I'm confused, I thought these were going the other way. E.g., there is a mismatch between what the spec calls an attribute and MLIR does, previously we had mostly err'd on treating them the same. But as the spec really requires this before execute/offload rather than representationaly , I thought the conclusion was that these should be operands and one had like a verify for export/runtime pass to flag that the operands are actually constants at that point.

Let me know when ready and I'll take a look (I didn't check failure)

Nice! Given this is very exact lowering required for numerics the test is fine, else I'd be conserned too much of change test. (when I see such lowering I keep thinking I want a couple of convenience wrappers ...)

Looks good, as Mehdi mentioned using Fixes #60069 will result in closing the issue upon commit.

I'm actually surprised we didn't have this one, thanks, let know if you need help landing.

Nice, thanks. Would you need help in landing?

Nice graduation (gets us even a step towards introspection ... Well more pieces needed :-)). Have you thought about exposing these C side?

Ah seems I never hit submit yesterday. Same comment as Alex on one and general question on verification part.

SG, could you add a TODO here documenting this so that we know when we look at it again?

[not a statement of endorsement of either form]

Could you perhaps expand on that exception or point to where this is assumed? Are these where during a pass an invalid state is created or is this across passes?

The change was originally motivated by avoiding the breakage when result type inference was being introduced. It has landed a while ago, so this change will now create the breakage it sought to avoid.