RFC: Collapse llvm-external-projects into the main source tree

[Stella Laurenzo]

I was just going to send a PR for this but thought I'd ask first: Any objection to collapsing the llvm-external-projects/iree-dialects subtree into the main compiler/src tree?

This presently has three dialect/other things in it:

• iree_input
• linalg_ext
• linglg_ext transform extensions

In the before times, there was a thought that these pieces might be useful to share outside of the project verbatim via the llvm external project mechanism. However, I'm not sure this really panned out as a useful thing, and there is a non-trivial amount of complexity cost associated with keeping them segregated in the tree/build this way.

LinalgExt used to be shared with the sandbox and we used to think it and iree_input might make their way into torch-mlir in some way. The former is no longer important and the latter had the dependency switched the other way.

I expect that the more important thing, regardless of where they live or are built is that the dependency dag remain clean for them. If that is the case and (say) someone wants to externally use the input dialect, it is a simple matter to export a copy of it and overlay some really simple build rules. This is what we do for inbound dependencies these days, and it has worked out and been the much simpler way to go (vs maintaining project structure forks).

Happy to take a different approach on this if anyone is using these in a way that is highly dependent on them living where they are and having the build exactly as it is.

- Stella

[Scott Todd]

We had a related discussion a few months ago on Discord, where I proposed a smaller change - keep dialect definitions in the external project and move the C++ passes and other code into the core project.

I'm in favor of either a partial simplification or full move, though I would like to minimize churn if possible. The long term benefits of simplifying that part of the tree would be nice.

We did change our usage of that directory in the downstream Google mono a bit (cl/560828283 for Googlers, 2 months ago). I'd almost want to hold off on big structural changes while we untangle things downstream, but I'm not sure what the timeline is there (Julian / Jacques could weigh in). I know that Eugene Z was also using IREEInputDialect for some explorations... not sure what the current status is there.

--
You received this message because you are subscribed to the Google Groups "iree-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to iree-discuss...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/CAEkedjiHZ8cEso2UR_D_QC1zH-_iVwOpOJ-CvmG9TkKVYkScfA%40mail.gmail.com.

[Mahesh Ravishankar]

Re: linalgext_transform extensions, could we first get the relevant folks to cull things that are not needed anymore. A bunch of things I think are obsolete or upstreamed...

To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/CAF-j_jeQ5X_XzOiRJOgDo%2BdkFURQHtW2eb1CQzgYP82DJRnLkw%40mail.gmail.com.

[Kunwar Shaanjeet Singh Grover]

+1 to this.

We were just having a discussion on Discord (https://discord.com/channels/689900678990135345/689957613152239638/1172493284652875816) about how llvm-external-projects behaves differently than the rest of IREE Compiler dialects. It is not tested by CI properly as it is built differently than the rest of the compiler and is currently, only tested by the ASAN CI path. On my machine, it also causes problems with clangd for me (could by an issue for me, but iree/compiler and mlir work great for me). Moving these dialects iree/compiler would be a good move in fixing these problems.

Best,
Kunwar

[Jacques Pienaar]

It would be great if some cleanups before integrate - normally I'd advise land and iterate, but we've had some open TODOs there for 2 years, so using this as opportunity to do those would be great 🙂 So +1 to Mahesh.

I did like the idea of input being "vendored" focussed. I'm not aware of where used (we were planning to do exactly that but plans changed, so not needed yet). I'm not sure it is easy to do without some more work. So keeping deps clean to avoid needing to do that later would be good. I'm also not sure (given a cmake dep in space 🙂) if one can't have both usage forms without much cost.

-- Jacques 

To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/0e2aa4e6-0e3b-4a3b-84bf-829b5cd0adc2n%40googlegroups.com.

[Stella Laurenzo]

I don't think we need to make such extreme project organization concessions to keep the dialect functionally what it is. I mainly want to reduce this being a disconnected attic that no one goes in to.

To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/CAM4W%2BYeg7t%2BsO21ADTn9cFAgtohTfipPj-PGnrL9-EAnwg2kww%40mail.gmail.com.

[Ravishankar, Mahesh]

[AMD Official Use Only - General]

Yeah, agreed. Id volunteer to do cleanup, but honestly wont be able to dedicate time to it. Moving things into Codegen would be a strict improvement. 

---

From: iree-d...@googlegroups.com <iree-d...@googlegroups.com> on behalf of Stella Laurenzo <ste...@laurenzo.org>
Sent: Sunday, November 26, 2023 9:58 AM
To: Jacques Pienaar <jpie...@google.com>
Cc: Kunwar Shaanjeet Singh Grover <grov...@gmail.com>; iree-discuss <iree-d...@googlegroups.com>
Subject: Re: [iree-discuss (public)] RFC: Collapse llvm-external-projects into the main source tree

 

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/CAH8pnHY8kq%3D7fTxqFpRgjeWhCKgJFQpgRz9x9gQnJ2377oNOGg%40mail.gmail.com.

[Scott Todd]

FWIW, regarding my comment earlier about wanting to wait a bit for the Google downstream repo to be updated, I think we're past the trickier parts there, so I wouldn't use that as a reason to delay these cleanups.

To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/SA1PR12MB7174EA5253B7790435B9885389BEA%40SA1PR12MB7174.namprd12.prod.outlook.com.

[Hanhan Wang]

I have been cleaning LinalgExt in the past few months. There will need more work, but I can see a path to collapse LinalgExt operations and tranformations into IREE's main source tree. The only question on my side is that where do we want to put the LinalgExt ops? They are frontend operations and transformations. I've been thinking them, and there are two options (welcome for other opinions) in my mind. One is https://github.com/openxla/iree/tree/main/compiler/plugins/input and the other is https://github.com/openxla/iree/tree/main/compiler/src/iree/compiler/Dialect. Does anyone have any preference about it? I can help drive this and layout the rest of work, if someone can answer the question.

I'm prioritizing this because it is making integration harder. I've addressed some issues in iree-dialects with others in recent integrates. The most obvious thing I can point out is that we have totally different transform dialect setup between IREE and iree-external-projects. I'd like to collapse it into IREE, so we can have less maintenance burden.

-- Hanhan

[Jacques Pienaar]

I think the latter seems a better fit.

To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/e29c2f86-d6d5-424a-b266-caf791a2cc44n%40googlegroups.com.

[Scott Todd]

Here are the current LinalgExt ops: https://iree.dev/reference/mlir-dialects/IREELinalgExt/. How many of those do you expect will remain?

An input plugin (or plugin in general) would be interesting, but I think that would only work nicely as a self-contained (and possibly optional) module that hangs new passes off of a few hooks exposed by the plugin API. I agree with Jacques - a new directory in the "core compiler" dialect folder feels closer to how LinalgExt is used.

[Laurenzo, Stella]

[AMD Official Use Only - General]

+1 to what Scott says.

To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/2486ced1-7962-4a2a-91e7-bc8693b6c820n%40googlegroups.com.

[Ben Vanik]

I think compiler/Dialect/ makes sense - note that I think it was being developed to be upstreamed with different conventions than we use under compiler/Dialect/ - if we move it in there we'll want to make it match the existing code.

To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/DM4PR12MB529596DC038C280B92187336E4752%40DM4PR12MB5295.namprd12.prod.outlook.com.

[Hanhan Wang]

> An input plugin (or plugin in general) would be interesting, but I think that would only work nicely as a self-contained (and possibly optional) module that hangs new passes off of a few hooks exposed by the plugin API. I agree with Jacques - a new directory in the "core compiler" dialect folder feels closer to how LinalgExt is used.

Cool, thanks for the input! I did not have any context about the input plugin. I've seen the directory, so I threw out an idea. It makes sense to me to put it to a new directory in the "core compiler".

> Here are the current LinalgExt ops: https://iree.dev/reference/mlir-dialects/IREELinalgExt/. How many of those do you expect will remain?

I want to move all of them out. But it looks like `IREELinalgExt_DoNotDCEOperandsOp` (i.e., transform.do_not_dce_operands) is used by the iree-dialect-opt transform dialect setup. So that will probably be the only one still living in llvm-external-projects/iree-dialects. The rest will be moved to compiler/src/iree/compiler/Dialect. I think the next step is breaking dependencies and moving the dialect to compiler/src/iree/compiler/Dialect. I will find some cycles for it. If there are works highly interacting with LinalgExt dialect, please let me know how we can coordinate it. So far I only see that https://github.com/openxla/iree/pull/16163 uses LinalgExt ops. I will coordinate with Max.

'Laurenzo, Stella' via iree-discuss <iree-d...@googlegroups.com> 於 2024年1月22日 週一 上午11:59寫道：

To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/DM4PR12MB529596DC038C280B92187336E4752%40DM4PR12MB5295.namprd12.prod.outlook.com.

[Hanhan Wang]

> I think compiler/Dialect/ makes sense - note that I think it was being developed to be upstreamed with different conventions than we use under compiler/Dialect/ - if we move it in there we'll want to make it match the existing code.

Good point! If I start the work, I will break it into several commit (but in one PR) like

1. Moving files to compiler/Dialect

2. Fix includes/namespace/whatever to make IREE compilation work.

3. Fix styling.

4. etc

I hope that will address concerns separately, and make review process easier.

'Ben Vanik' via iree-discuss <iree-d...@googlegroups.com> 於 2024年1月25日 週四 上午9:12寫道：

To view this discussion on the web visit https://groups.google.com/d/msgid/iree-discuss/CAJ5XP9z3tVpP2JGW_EnHg9EFPjMfzEPRVqPococou-XPqCYuLg%40mail.gmail.com.

[Ben Vanik]

thanks for doing this cleanup hanhan!

[Hanhan Wang]

Hey all, I have a prototype working now. The binding supports are removed in the prototype, and it does not trigger any issues. They were explicit exports, because they lived in llvm-external-projects. I don't have context about them, so I did not add them to the IREE main source tree. If people think that we should add them in the PR, please point me to how to do that. Please take a look at the PR: https://github.com/openxla/iree/pull/16407

Thanks,

Hanhan

[Hanhan Wang]

Thanks for all the reviews and input! I just landed the PR, now LinalgExt dialect lives in the IREE main source tree. We still need some cleanups in both IREE and llvm-external-projectt. I'll prepare PRs soon.