Breaking change to gpu.func to spv.func lowering

My understanding of abstractions is that they model different things with different requirements and they can be at different levels with different fidelity. MLIR’s great benefits allow one to choose a suitable level for a particular need. ABI related issues are typically at component boundaries so one often is interacting with some alien component that is not directly controlled by oneself. So I’m not sure how one can easily “abstract away”. But I might miss something here.

I’m not quite sure how to perceive this. Could you be more specific? To me, abstraction is a case-by-case discussion, even in the same dialect. One might want to model certain aspects with high fidelity but jump levels a bit for others. SPIR-V is a foreign/interface dialect that models an external entity. We’d like to stay at the same level mostly. But I think it’s not specific to SPIR-V here; all foreign/interface dialects has this characteristic: they map mostly 1:1 to the external entity. It seems one might extend the same argument here to say TensorFlow/LLVM/etc. is lacking abstractions.

I guess maybe it’s because how the this is used is not in core MLIR repo that this looks like “shortcut”. Mahesh included links to IREE use cases in the above if that helps to clarify things.

We map Vulkan/SPIR-V’s model with high fidelity because we’d like to expose and control these aspects in IREE; there are validity and performance reasons to do that as explained in the above comments.

For others who don’t care, we’ve discussed to introduce a default behavior that follows the sequential assignment and I think Mahesh has already implemented that in the proposed change. So from the GPU dialect and in-tree runner’s perspective, this should still work uninterrupted (without attaching the attribute). So this is not a breaking change anymore. (Sorry about the false alarming there if that’s the case.)

Oh, okay. If this is the real concern, then we can certainly revisit this point specifically. This needs more considerations over the trade-offs, but a quick thought might be embedding these attributes in an op, say, spv.mlir.interface. It can holds a SymbolRef to the function and be placed in the region of the outermost module op.

I wonder how one draw the line between experimenting vs non-experimenting? MLIR is still young and evolving fast and seeing new exciting iterations everyday. What we have in the SPIR-V dialect are certainly not casted into stone. And personally I’m quite happy to evolve towards better solutions there. :slight_smile:

There are a lot of good points in the three way discussion regarding design philosophy but I wonder if it wouldn’t be better to discuss separately and more directly sometime? I’ve spent some significant time chatting with each of you one on one about some of these things, and I think there are definitely some higher level alignments that could take place (which I want to see happen but also want to see the specific technical feedback in this thread addressed).

I don’t know what you want to say.

I’m not sure what you mean either here. ABI are abstracted away all the time (frequently imperfectly but still).
For example the C++ language does not specify how to lower code like this:

struct xyz {
  int a[4];
int struct_arg(xyz f) {
    return f.a[0];
int struct_arg_caller() {
    return struct_arg(xyz{});

And the compiler does not need the user to provide an attribute on the function to tell that on x86-32 you need to pass the struct on the stack as a pointer while on x86-64 you need to pass it by value here:

If we were not building these abstractions, we’d expose all the details of every platforms to every end users. I see these abstractions are just fundamental to compiler development in the first place.

By not providing abstractions, you’re just pushing the complexity to the next layer of the stack.

If you don’t preserve the property I mentioned before (the mapping from arguments between the launch and the kernel is the one “source-of-truth”) then you haven’t changed the fundamental problem here.

I’d point out that the comment in the revision is far from re-assuring at the moment:

This is more a escape hatch and is legacy. When I initially set this up it was convenient, but this is very limited in its usage. I would actually prefer to not have this, and any realistic user of the SPIR-V lowering will not be able to use this on many devices. 

I am not sure how is this supposed to be re-assuring in any way?
I’d expect that we work to provide a coherent/consistent story for targeting GPUs in general in MLIR. The GPU dialect is supposed to provide this abstraction, and you’re motivating breaking this abstraction by optimizing closer for IREE?

The change discussed here is not purely about the design of the SPIR-V dialect in itself, but is really about the conversion of the GPU dialect to the SPIR-V dialect. And so the discussion in my opinion is fundamentally about the definition of the GPU dialect and the abstraction that it is providing.

This thread is quite off topic at this point, and to be honest, I find it fairly uncomfortable to participate in. I have plenty of input that I think would be salient to a discussion of the design but I am unlikely to speak up about it due to the confrontational undertones. I suspect that I am not alone in this and we are not doing this topic justice by communicating in this way.

In exchange for me not providing one “last word” on the discussion in this thread, would you all be willing
to do likewise and, next week, to reset and start unpacking these points systematically and with better inclusivity? Everyone is under a lot of stress right now, and I think taking a breath and re-approaching might keep us from doing damage to each other under what are really challenging circumstances. I speak to all of you 1:1, and I assure you that you are all operating with the best of intentions. So maybe start there next week, reset, and unpack further in a more neutral setting?

(If I’m misreading and you all are just having a bluntly productive conversation, feel free to say so and bypass my opinion)