RFC: move Vararg specType rewriting into dispatch

[vtjnash]

As a start to addressing the need for a specSig representation for vararg calls, this moves the responsibility for forming the varargs argument tuple type from the callee to the caller (e.g. dispatch or specsig). My thought here is that instead of implementing this logic in codegen as a special case, this representation will make it easier to share existing code for allocating and passing around tuples. This means that now we would now have that length(specTypes) == length(argnames). I think this representation (where specTypes maps directly to argnames instead of to the callee's argument tuple) is cleaner than having each consumer effectively re-implement this each time (inference, codegen, generic functions for #12783)

this implements #12783 along the way

[timholy]

Will this cause the allocation of a tuple for each call to a varargs function?

[vtjnash]

no, that's what we do now (where the callee is expected to make the tuple). this instead modifies it such that dispatch is expected to create the tuple (which for specsig, would be inlined into the caller)

[JeffBezanson]

To me that seems to involve tuples in more of the calling process, making them harder to remove. Of course, in the specsig case the caller already has full knowledge of the callee, so it doesn't make much difference. But outside that case the caller doesn't know it's calling a varargs function, so it doesn't quite seem right for it to be the caller's responsibility to handle it.

[vtjnash]

right, the "caller" is dispatch (or specsig, after inlining a specialization). this changes is intended to make it so that passes that consume a LambdaInfo don't generally need to consider whether a signature is varargs – the types of the arguments simply line up with their names (and slot numbers).

[tkelman]

CI is failing to trigger here oh it's a PR into a PR, nevermind

[timholy]

Does this include a rebase? It's got the whole #11242 set of patches, too, despite the fact that it's against #11242.

Speaking for just myself, I'm fine with you merging this whenever you feel ready to do so.

[JeffBezanson]

I don't really see why this is needed for #11242.

I'm also not convinced this is a good idea in general. The core issue is that, currently, every argument tuple type we have refers to the caller's arguments. We need to form these tuple types for dispatch. However, once the arguments are bound to names in the callee, there isn't any need for a tuple type of those. Sure, you can represent any N types with a tuple type, but there is no need for this tuple type itself, which makes it suspect to me.

One symptom is that the result of ml_matches can no longer be understood as a type intersection. It's type intersection plus some rewriting to form a vararg tuple type.

Another symptom is, imagine we want to specialize f(x...) for 3 Int arguments by compiling it the same way we'd compile f(::Int, ::Int, ::Int) today. If we did that, there would never be any tuple involved, so there should be no corresponding tuple type. This is a decision entirely about the compilation of a single method, that should not be exposed outside it (until you get to the very last step of calling it with a specialized signature). So it seems like the wrong abstraction to me to say that you need to form NTuple{3,Int} as part of looking up this method.

[vtjnash]

Changing specTypes isn't essential, since we can unambiguously recreate this svec at any point from sig + nreq. However, after dispatch, we also generally don't need to know sig, we need to know the types of the arguments. And so this change would make that information more readily available.

Another symptom is, imagine we want to specialize f(x...) for 3 Int arguments by compiling it the same way we'd compile f(::Int, ::Int, ::Int) today. If we did that, there would never be any tuple involved, so there should be no corresponding tuple type

where did x go? you can't simply just delete a variable and then claim that that there was never a tuple involved. The type of x is still NTuple{Int, 3}, even after you sunk it into the function. This argument is also completely unrelated, since what codegen decides to do with ::Int, ::Int, ::Int is not defined by a particular transform on specTypes, although the general idea is of course to pick it such that it does reflect the expected usage transform. Since often we need to forward or iterate this argument, I believe that passing it as NTuple{3, Int} is likely to be far much more efficient (iteration generally requires spilling to memory) and also generalizes better for NTuple{N, Int} where N is either large or unknown (and is thus passed separately, but the ntuple can still be unboxed).

One symptom is that the result of ml_matches

yeah, i'm not convinced of the value of that transformation timing myself either. it would probably be just as sensible for the caller to continue making that transform if it cares. (e.g. how it happens in type-inference now, which I deleted after moving the transformation into ml_matches)

[JeffBezanson]

where did x go?

It's just that x only exists inside the function. Indeed, it would be possible to transform the function such that there were 3 arguments x1, x2, and x3 instead of x, and the caller would not know the difference. I don't think we'll actually do this, but it clarifies the ownership of this information.

[vtjnash]

but it clarifies the ownership of this information

except that it doesn't: specTypes is consumed by the callee not the caller. I see no reason that f(x...) and f(x,y,z) need to have the same calling convention, when I think it is likely more efficient for them to use different calling conventions. (and as you pointed out, there's also no particular reason for specTypes to be a tupletype, it could equally well just be an svec, since all the consumers really end up caring about is the types of the argument variables).

[timholy]

Jeff's strategy is the one I took in my own attempt at this, #11242 (comment).

[JeffBezanson]

I don't necessarily advocate doing that transformation, I just feel like this increases coupling. Actually I really like the idea of renaming specTypes to argTypes and making it an svec. That greatly clarifies the intended meaning here. From there we just need to be a bit careful about whose responsibility it is to initialize that field. ml_matches is probably too early.

As an interesting data point, length(Tuple.name.cache) is ~26000 on master and ~39000 on this branch (the cache grows by 1.5x so this doesn't reflect the actual number of types). That itself is not really a problem, but it does indicate that this introduces a fair number of unnecessary tuple types. Another reason to switch to an svec.

[JeffBezanson]

Actually, we could store these types in slottypes and remove the specTypes field entirely.

[vtjnash]

Actually, we could store these types in slottypes and remove the specTypes field entirely.

we would need to change lowering such that if an argument is reused as a variable it gets two slots, but i think that's certainly feasible

[JeffBezanson]

Yes, it might be better to explicitly introduce new slots for assigned arguments, since codegen basically has to implement that anyway.

[JeffBezanson]

#16165 Looks like a case where it would have been more convenient for lowering to have done that transformation.