I like structure too as it captures information about the structure of the values to me. encoding makes me think jpeg or Levensthein distance and the like.

I think this looks good to go and we can always iterate

Thanks!

This seems as expected (capturing what is known in the non-error case when building), I'm surprised no tests needed updating - guessing as we don't have one exercising this builder. Could you perhaps add a small C++ unit test for it?

I'd prefer something like

Nice, thanks

Nice, any performance tests you did?

Good catch

Did https://github.com/llvm/llvm-project/commit/113baa2b9fd3c8db30d33ecc3f068af48dcce52d address this?

LG, thanks

Thanks!

Looks like a nice cleanup, I see:

LG to me interface wise modulo some ergonomics and questions others have there. Thanks!

In D97887#2621515, @nicolasvasilache wrote:

I have not yet dug into the guts of the discussion and the implementation details, I'll just comment on 2 things:

There was push back against using OpFoldResult instead of Value. From experience the use of OpFoldResult is really important.

Where it makes sense, it is crucial to move towards a ValueOrAttr abstraction rather than stay stuck in Value land.
Maybe it does not (yet?) make sense for the Shape world and that is perfectly fine.

I'll probably need to go through these again later, but looks good from check and can be refined later.

Nice, thanks

Nice, thanks

Adding Stephan as I'll be OOO

Nice, thanks

Thanks this will help a lot :)

Thanks :)

In D97622#2606872, @rriddle wrote:

Is there more planned here? Seems like uses of this trait right now will result in double verification + double code size?

Added heading and expanded comment

Nice improvement, thanks

It's fine to have these separate for review, but please merge to avoid updating the interface in two consecutive changes where one undos part of the previous

Nice!

In D93085#2538095, @mehdi_amini wrote:

I forget, what was River's suggestion to make this more efficient?

There wasn't an efficiency aspect left, the idea River had was about generating a different base class with TableGen so that the subclass implementation for the interface fallback could be friendlier to write.

Thanks!

Nice, thanks for the super fast improvement!

Is this something that we want to make a convention, then we'd want to then capture in https://mlir.llvm.org/getting_started/DeveloperGuide/ (I don't believe it is a LLVM convention)

LGTM, I'll leave more to Tres who knows these better

Thanks, reverted,

I know River had a better/alternate approach in mind here too

Only access union element that was set for case

So this will avoid that long pause at the end?