Note: the absence of useRecordFunction here introduces an API inconsistency between callBoxed and callWithDispatchKey - one lets you control RecordFunction and the other doesn't. If we go with the useRecordFunction approach, I'm thinking we should add it to callBoxed as well (we can avoid callsite churn by defaulting it).

But also, if you're inclined to dislike call-site-decides because of leakage, here's some more :P

I was wondering in what cases callBoxed would be called with BackendSelect, or is it for the symmetry

Yeah, just for the symmetry. Unless there's some history that I'm missing, there's no reason to think we don't want the same knobs on boxed calls as unboxed.

Note: I wouldn't necessarily burden your PR with the addition - I just wanted to note it in the context of the discussion about filtering BackendSelect et al.

I think you can put the dispatch key filter test inside this if, and also move the op.isObserved() test down here.

@ezyang @ilia-cher to tie up the points that Ed raises here:

• pushing a string means a dynamic allocation: this is true, though we'll stop incurring the cost once everything is supported. So I'd probably let it go in, unless there's a super easy way to push it in a non-owning way, like make "cannot box" a static std::string and push references to it or something. cc @smessmer any obvious way to do this?
• move these functions closer to the callers: this is another case where it's a matter of how much work to do for a temporary situation. These functions use locally-defined templates that shouldn't really be public, so I think we can treat these functions as ephemeral bits of the API presented by boxing.h to the rest of the system.
• naming: I agree it's a little confusing rn, since boxArgumentsIntoStack sounds like it performs a function that's actually done inline in boxAndCallBoxedFunc, but in fact does something subtly different :D ...but it's not too big a deal, and I can tweak names in my upcoming PR. Ilia if you're pushing another commit anyway, you might consider renaming boxArgumentsIntoStack to be boxArgumentsOrCannotBoxIntoStack to clarify what's happening, but otherwise I wouldn't worry about it.

Hmm is there a reason to break symmetry here? I think we want redispatch to be exactly equivalent to callWithDispatchKey on the computed key - basically just a convenience method. Introducing this logic change seems like a potential source of hard-to-track-down inconsistencies in recording results.

If the change is to avoid double-counting some key other than BackendSelect, maybe that key should be added to shouldRecord? cc @smessmer maybe you guys have talked about this already...

the reason I do it this way here is that I don't want to run RecordFunction and we don't want to pass a parameter into callWithDispatchKey (as we discussed). Since it's two lines of code I just called the kernel itself

Yeah, I definitely have no problem with the implementation. My reaction was to the semantics - my default assumption was that we want redispatch(op, args) to have the exactly same behavior as the equivalent callWithDispatchKey(op, key, args) call - w.r.t. RecordFunction and everything else.

But the other POV makes sense too - that a natural consequence of the meaning of "redispatch" is: no (re)recording, so they really are different.

It's not totally clear to me that the suppression shouldn't move into shouldRecord, but it's not a discussion we need to have before you land this 😄 thanks for being patient!