Address review comments.

CMake

Slowly getting back to this after vacation, I'll rebase on top of an NFC change so people see less noise.

LG!

Cool, thanks Alex!

Thanks for adding this Aart!
Made a quick pass as I am catching up.
I am wondering whether the impl could be simplified or whether some of the complexity is a fundamental property of the problem at hand?

Marking as blocker to be sure my question is addressed, sorry for being late to the party as I was away last week.

Are we sure we want another parallel SW stack to do fusion here?
I'd love to understand the use cases that are fundamentally different from what we'd expect to do with Linalg + Affine (with or without Intel's multi-for).

I wonder how much of the test here is just carried over mechanically and if we could do something to reduce this churn...

we didn't really get an answer to the questions. I also mentioned

In D73826#1877588, @mehdi_amini wrote:

Is there a world in which we can have such an Einsum-based parser on textual format to specify Linalg ops?
Does this have to be a language outside of MLIR? (I would certainly hope not).

I don't understand how these two questions can go together as they seem entirely disconnected to me, so I'm probably not understanding the questions in the first place :)

I think https://reviews.llvm.org/D73405 sets a nice precedent and I would very much want to see something like this for specifying Linalg ops declaratively.

The main reason I have been reluctant to move on here is exactly because I would very much want to see something like this for specifying Linalg ops declaratively!
Where "something like this" is a feature that is generic enough to generalize and be reused across dialects, like D73405 :)

@jpienaar and @mehdi_amini thanks for your fast turnaround on this!

Corner cases are the worst :)
Thanks a lot Mahesh, very cool!

@dcaballe I don't think reverting the commit will be a reasonable path here, a bunch of other things and integrates have switched to this.

Minor nits, approving conditionally.
Thanks Aart!

Address review comments.

Update CMake.

Finish the revision and update CMakeLists.txt.

@flaub so actually, one of the (multiple) reasons I am pushing for this is to have the ability to add a generic contraction op and a generic pointwise op as special cases :)
We already have vector.contract added by andydavis@ with similar semantics and it would make the rewrite to vector form trivial.

Thank you!

@mehdi_amini @jpienaar @rriddle @ftynse @antiagainst I have rebased and updated the revision description as well as the impl.
I would be interested in your opinion on where this is going and if it matches the expected behavior you may have.
If you see some red flags please let me know, I hope to have this working and pushable by tomorrow.

Continue the impl.

Rebase on top of NFC changes and spin out a separate tablegen backend.

Fix build breadking typo.

ld breaking typo.

ddress review comments.

ddress review comments.

Thanks for pushing on this, LGTM!

Format.

There are unsolved issues going from the buffer world to the tensor world when reductions are involved.
Changing the semantics of all the ops, including ones that don't have reductions, is not the right way to go here.
We need a proper way to tie a tensor result to a tensor operand.
We don't have a good mechanism for this atm and this should be discussed with the bigger group.

Nice pattern composition Andy!

My apologies for the delay..
Also, please note that https://github.com/llvm/llvm-project/commit/75394e1301dd1d4739a24d440e9996c52e5b5570#diff-6635fa1a5b0b10edf6e511585c7c986d refactored things significantly on the EDSC front so this would need a rebase.

Flushing out some answers.

Address review comments.

Address review comments, revive CMake changes.

cmake seems broken

Address review comments.

Addressed.

@rriddle I did not see a simple way to untangle things and once I got things working I did not look back.
For instance, dropping dependencies from EDSC onto LoopOps and AffineOps requires extracting the makeLoopBuilder into a free function.
The only place where it makes sense to put this is in the loop dialect.
There are a bunch of similar cases.
At which point I don't see a reasonable way to split this up.

Ugh .. misread the green checkmark from Harbormaster as the second approval from @mehdi_amini post trivial rebase.
As a consequence this landed in "Needs Review", sorry about that ...

Rebase.

Thanks Mehdi!

Fix bad rebase.

Thanks for adding this test Hanhan!
I am somewhat less confident that linalg.indexed_generic generally works (since there didn't exist tests before this one, my thinking is it is not yet functional), but this first test is a concrete step forward.

Thanks @nmostafa for pushing on this important feature, it will make everyone's life better.
Looking forward to seeing this lower to LLVM all the way to execution!

I question the relevance of carrying implicit broadcasting concepts from HLO and other frontends into MLIR core.
I see this as a red flag that the required separation of concerns has not happened.
Anything related to broadcast should be on the XLA / frontend side IMO.

Thanks Aart!

Rebase on top of llvm.intr.fma addition.
Address review comments.

This looks really great, thanks a lot for pushing this Mahesh!
I think it is an elegant solution that shows how proper usage of the StructuredOps abstraction avoid creating problems that should never have existed in the first place (e.g. carrying broadcast and all the phase ordering issues related to buffer allocation, fusion and erasing unnecessary buffers).

The above functionality could already be handled with OpInterfaces + the way export generation is handled for TFLite dialect [which doesn't use OpInterfaces as that predated it, but could], one need not change OpDefinitionGen to get this behavior.