Re: [tclquadcode] Defaults cause confusion on dkf-callframe-impl branch

[Kevin K. <kev...@gm...>]

Replying on-list to a private message, having elided irrelevant personal
communications:

On Sun, May 28, 2017 at 3:10 AM, Donal K. Fellows <
don...@ma...> wrote:
>
> The thunk generator right now will do calls using STRING arguments, with
> all defaults supplied (also as STRINGs). I used to do more complicated
> things, but I'm pushing that into your side of the world now that we don't
> need to generate a single type for each procedure.
>

That's almost the right thing. I'd prefer, if possible, that defaults be
supplied as [quadcode::typeOfLiteral $defaultValue], but
considering all the other costs associated with going between interpreted
and compiled code, that's rather a nit.

In the case I'm reporting, the problem call is to [cos] from either one of
> [coscaller1] or [coscaller2]. They have both decided that the [cos] they
> wish to call is the [cos] that takes a single DOUBLE argument, and the
> specializer has asked that this additional version be created, but the
> quadcode for that version still internally asks for the second parameter.
> LLVM (understandably!) chokes on being asked for the second parameter to a
> one-argument function, so I need an alternative approach.
>

I'm starting to have a look at that now. I'm guessing that somehow we're
committed to making a one-arg [cos] before the
[varargs] pass has had a chance to supply the default. In that case, it
would not surprise me if nobody calls it, because
[varargs] ought to rewrite the call site. There's something broken in the
plumbing, and this should be an easy fix.


> It also asks for it as a STRING, and that's inefficient. My thinking is
> that there ought to be a pass that checks whether a parameter's index is
> one that is made available in the actual current argument list, and if not,
> replaces:
>
>    param {var X} {arg INDEX-Y}
>
> with:
>
>    copy {var X} {literal DEFAULT-Y}
>
> The mapping info to do that is extracted into the bytecode dictionary; it
> should definitely be available. The pass should also throw an error if
> there's no such default possible I suppose, since the generated code will
> definitely be impossible to issue correct code for in that case.
>
> But that's assuming that the specializer is right to ask for a
> single-DOUBLE-argument [cos] in the first place. That's your call. ;-)


Whatever is happening, it isn't right.

Where are we at present with callframes? I'm trying no to commit
>> anything to your callframe branch except for trunk merges, and only when
>> there's something that one one of my branches needs. My little brain has
>> trouble coping with tje merge pattern otherwise. I'm now wondering which
>> tests I should be enabling.
>>
>>
> I'd love to figure out which branches you think ought to be merged. I
> don't mind doing the leg-work. Well, finger-work. :-)
>

Right now, I have a cascading merge pattern going, which I don't mind that
much, but I think we're getting a bit out of step.

|> Trunk - I try to maintain this as 'all tests pass' always.

|=> kbk-calframe - Abandoned branch, if I've not closed it, I ought to.
Everything worth keeping from here is merged into dkf-callframe-impl

|==> dkf-callframe-impl - Now merging from trunk rather than kbk-callframe.
I'm nervous about making substantive changes here, not knowing what you're
doing (or which tests I should be expecting to pass).

|===> kbk-callframe-motion - This has some significant optimizations
removing moveToCallFrame and moveFromCallframe for local variables that are
known to be untouched by the called command. It's a fairly minor change.
Assuming that it doesn't cause trouble, it's ready to land into
dkf-callframe-impl, and I'd like to have one fewer branch to cascade.

|====> kbk-namespace-variables - This branch depends on
kbk-callframe-motion, and is where I'm sketching out how [global],
[variable], [namespace upvar] and possibly plain [upvar] will have to work.
I'll talk to you on-list soon about aliasing, and about planning ahead for
resolvers. (I don't claim to understand them, so I'm going to need help
here.)


> The aim is for all test cases to pass. Unfortunately, some of those test
> cases are expensive to compute a baseline cost for (as I've not yet done
> any form of auto-calibration of costs), so it's usually a good idea to
> specify a smaller number directly. If I'm not hunting refcount handling
> bugs, I often use a small prime number of iterations (29 seems reasonable
> to me) as that encourages a reasonable amount of information to be produced
> by the timing routines. Remember also that those timing routines are
> repeated several (5) times as well, in order to try to avoid effects due to
> interruptions by the OS, and there's also separate code to try to analyse
> the result values and pre-warm any caches. All of which is good, but makes
> testing a more complex script expensive.


> I'll often use targeted testing (using the -just command line option) just
> to speed up the turnaround time or reduce the amount of output down to a
> level I can cope with, but the goal is always “everything”. Fortunately,
> for this case we can see the problems with “-just cos” so it isn't
> difficult to hunt.


I was more asking about what set of tests I should be expecting to pass
just at the moment in the callframe branch - which were bugs that I should
be hunting and which were the result of incomplete implementation in the
code issuer. Now that I know about the ::cos bug, I'll go trying to swat
that one.