Failing tests in latest LLVM: mlir-cpu-runner: CommandLine Error

Yeah, I think by virtue of it being the only minimal, in tree way to do such things - that ship has sailed on it being a toy (as @bondhugula notes). We should fix the layering by copying the code it needs (and factoring out needless use of conveniences).

Aside from that, I’m a bit concerned with the name squatting it is doing: the reason a toy like this is the only thing in tree is because principled async behavior is both hard and domain specific to do in detail. Not only is there a lot of work being done out of tree to find and productionize the right abstractions (I am aware of both IREE and TFRT’s approach), it seems really unlikely to all converge to this one way at this one level.

I’m supportive of having something basic like this in tree because it is plainly useful, but I wish it didn’t have a name that implied exclusivity to the range of options that will someday exist.

There are two things: the runtime implementation, which is a toy and does not really have a load-bearing name, and the dialect and other compiler facilities, which aren’t at all in the “toy” category (or, no more than most other dialects upstream).

To come back to the original topic and issues with libSupport being hostile to reuse, I just sent ⚙ D105959 Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer which may help.

3 Likes

niiice

I don’t think we need all to converge on the async dialect for it to be useful. Having a shared, relatively unopinionated dialect to model async behavior in tree is useful to identify common interfaces and analyses. And it is in use in various places that do not have complex needs. Sometimes IREE and TFRT are too big a hammer for the problem.

When that day comes, we can think about better ways to do naming for dialects (Java style package names anyone? :slight_smile: ) but until then, I don’t see name squatting as a big issue here. Also, what would the alternative be? simple_async?

1 Like

Yeah, I agree, and my dissonance is completely about naming – so at least I’m hung up on the hardest issue in computer science :slight_smile:

I think if I mentally rename this to async_await then it makes more sense to me and models exactly what it says on the label. Not that we need to do anything now (but maybe I’ll send a patch to clarify the dialect documentation), but my reasoning is that “async” covers a broad class of things (unless if in the context of a specific language which defines it concretely in a way that we probably don’t want to do in MLIR), whereas “async_await” is specific and unambiguous with respect to what it is and isn’t. Since you mentioned Java, I’ll say that I’m somewhat twitchy about such things growing organically (ala. java.util, java.util.concurrent, the myriad commons libraries, etc). Lived through one too many years of that :slight_smile:

Wow, thanks for the nice solution, Mehdi. You are singular in your ability to get rid of globals :slight_smile:

So, this makes libSupport static clean and safe to use as a pure library. I wonder if there is some kind of test we can devise to ensure it stays that way?

1 Like

The compiler flags Mehdi used there should flag if this changes I believe.

I would think this will be enough: Build libSupport with -Werror=global-constructors (NFC) · llvm/llvm-project@1f71bca · GitHub ?

(this patch didn’t stick: std::mutex wants a dynamic destructor with libc++)