Discussion:
[m-dev.] New pragmas for tail recursion warnings
Zoltan Somogyi
2015-11-13 06:46:36 UTC
Permalink
What I'd like to have reviewed at this stage are the names and syntax of the
pragmas.
% This pragma works on all procedures of a given predicate/function.
:- pragma no_warn_tail_recursion(NAME/ARITY).
% This pragma works on a single procedure.
:- pragma no_warn_tail_recursion(NAME(ARG_MODES*)).
:- pragma warn_tail_recursion(NAME/ARITY).
:- pragma warn_tail_recursion(NAME(ARG_MODES*)).
Argh, I meant to say no_warn_non_tail_recursion and
warn_non_tail_recursion, these match the option name
--warn-non-tail-recursion.
The predicate parse_arity_or_modes in prog_io_pragma.m
should parse either the NAME/ARITY or the NAME(ARG_MODES) syntax
for specifying a procedure or set of procedures. In the following,
I use PROC_SPEC as a shorthand for either.

I don't like the double negative in the name no_warn_non_tail_recursion.
I think it is very likely to confuse people, and even the ones who aren't
confused will not like having to take time to resolve the negatives.

I propose something like this:

:- pragma require_tail_recursion(PROC_SPEC, [OPTION_1, ... OPTION_N]).

where an option may be "warn" or "error" to govern what happens if
the requirement isn't met, or it may be "none", "self_recursion_only",
"mutual_recursion_only", or "self_and_mutual_recursion" to say what
kinds of recursion are required to be tail recursion, and maybe
"clique([OTHER_PROC_SPEC_1, ... OTHER_PROC_SPEC_N])" if the
user wants to specify what procedures the clique should have, so they
get a warning or an error if the clique accidentally becomes bigger
than what they expected.

That leaves the question of what the defaults should be; I propose
"warn", "self_and_mutual_recursion", and no of course no explicitly
specified clique.

We could also add an option that lists the backends on which
the requirement is valid.

Both your proposal and mine has to decide what happens if the
specified predicate has no recursive calls at all, and what happens
if it contains a higher order call (which may end up being a recursive
call, if the procedure's address is taken). I am pretty sure we would
have to ignore the latter, but the former is a different question.

Zoltan.
Zoltan Somogyi
2015-11-16 03:00:02 UTC
Permalink
:- pragma require_tail_recursion(foo/3,
[warn, self_recursion_only,
llc_backend(self_and_mutual_recursion)]).
Can you imagine that ever being useful?
I imagined this because they always want a warning about self-recursion
that's not tail recursion, or on the low-level C backend where it's
supported, self and mutual recursion that's not tail recursive. Effectively
suppressing the warning about mutual recursion on backends that don't
support mutual recursion. That maybe something the user would prefer to
control wholesale using a compiler option.
You haven't answered my question: WHY would they want that? Why
would they think that such a warning would be useful to them? (See below.)
"can this code work in constant stack space?". If the "code" in question
is mutually recursive, having a backend that guarantees tail recursion
for self recursive calls only is not much help. Even if half the tailcalls
are self-tail-calls and the other half are tail calls to other procedures
in the scc, optimizing only the former will still cause you to blow the
stack with large enough inputs. The only thing that changes is that
"large enough" is now (on average) twice as large. Is that a useful
amount of help? Probably only in pretty rare circumstances.
It depends on why the programmer wants constant stack space. This is the
main reason, to avoid blowing the stack. However a user may simply want to
reduce the amount of memory touched and therefore memory bandwidth required.
But you're right, almost all of the time the programmer wants to avoid
blowing the stack.
I guess the question is, if they take some code that uses mutual and direct
tail recursion, and run it on a backend that supports only direct tail
recursion, do they want to suppress the warnings for the mutually recursive
calls that are no-longer tail recursive, even if it means that they may now
blow the stack for large inputs?
My point is: a predicate is either self-recursive only or is part of a mutually
recursive scc; it does not change from one to the other just because gets
recompiled on a different backend. And the user either wants to be told
when the backend cannot guarantee that the predicate can be executed
in constant stack space, or they don't want to be told. I don't see users
ever saying to themselves "This predicate is mutually recursive, so it will
blow the stack unless the backend supports both self and mutual tail
recursion. Nevertheless, I don't want to be warned about this fact if I am
compiling this on a platform that supports self-tail-recursion but not
mutual-tail-recursion." Likewise, I don't think any user will say "this predicate
is only self recursive, but I want to be warned about it on platforms
that support self tail recursion but don't support mutual-tail-recursion."

A compiler option applies to all predicates in a module, some of which
may be self-recursive and others mutually recursive, so this consideration
does not apply to them.

If you are worried about a predicate changing e.g. from self-recursive
to mutually recursive when the programmer updates its code but
forgets to update the pragma, that is a legitimate concern, but I don't
think your proposed design helps to address it. That would be better
handled by extending the none/tail_only/etc spectrum to include
an option that would allow the programmer to say "please warn me
if the compiler can't make all the recursive calls in this predicate tail
calls, and I leave it up to the compiler to work out whether those
recursive calls are self or mutual recursive."
Post by Zoltan Somogyi
Both your proposal and mine has to decide what happens if the
specified predicate has no recursive calls at all, and what happens
if it contains a higher order call (which may end up being a recursive
call, if the procedure's address is taken). I am pretty sure we would
have to ignore the latter, but the former is a different question.
Of course the latter case might only be directly recursive if it has the
same type or a curried type. It may however be mutually recursive. It may
also happen if an ancestor's address has been taken.
Exactly. The ancestor may not even be in the current module,
which is why I don't think the compiler can do anything useful about it.
Yep, I would therefore not warn at all for higher order calls, including
method calls.
Hrm, if ho-specialisation is applied that may be different. We could safely
warn about them then, however the user may be confused by the
specialisation.
Agreed.

Maybe we should introduce new scopes in the compiler that would record
that the goal inside came from the inlining of a given call, recording the
call's details (location of the call, the callee's name, and the actual argument list),
or that the goal inside came from higher order specialization of a call,
again recording similar details. They would have no effect on code generation,
but they would allow you to generate better error messages.

An issue I can see with that is that we would have to decide in advance
exactly what invariants such analyses would be allowed to assume about
the scopes, so that code in the compiler that moved code across the boundaries
of the scope could ensure that it did not invalidate the invariant. I say in advance
because changing the invariant later would be almost impossible; since the
invariant would not be about the HLDS itself but about its history, you couldn't
just look at the HLDS and test whether the invariant was satisfied. Therefore
if you changed the invariant to make it tougher, there would no way to
automatically test whether compiler optimizations broke the updated invariant.

Zoltan.
Paul Bone
2015-12-16 12:15:16 UTC
Permalink
I thought of a useful addition to this proposal.

Currently both the pragma and --warn-non-tail-recursion will emit a warning
for the second recursive call in quicksortapp but not the first.

qsortapp([], []).
qsortapp([Pivot | T], List) :-
partition(Pivot, T, [], Left0, [], Right0),
qsortapp(Left0, Left),
qsortapp(Right0, Right),
append(Left, [Pivot | Right], List).

This is intentional to prevent the output from being too noisy. It's also
pretty obvious, that the first call isn't a tail call. Even when it's not
always obvious that the second call isn't a tail call. Here it looks as if
the last call is qsortapp, but it's not because of the call to ++ in the
clause head. Construction and changing argument orders can also cause
confusion, which is part of the motivation for the pragma and command line
option.

qsortapp([], []).
qsortapp([Pivot | T], Left ++ [Pivot | Right]) :-
partition(Pivot, T, [], Left0, [], Right0),
qsortapp(Left0, Left),
qsortapp(Right0, Right).

Anyway, I'm trying to show that omitting the warning for the first recursive
call is good.

However when we make the second call tail recursive by using an
accumulator, in qsortacc, this feature can be a problem:

qsortacc([], Acc, Acc).
qsortacc([Pivot | T], Acc0, Acc) :-
partition(Pivot, T, [], Left0, [], Right0),
qsortacc(Right0, Acc0, Right),
qsortacc(Left0, [Pivot | Right], Acc).

The second recursive call is a tail call, so it gets no warning. However
the first recursive call is not a tail call, which should be obvious.
However when the compiler doesn't generate a warning, and the developer
doesn't look at the predicate body at all, they might conclude that this
predicate uses constant stack space.

Depending on what the developer is thinking it may either be useful or a
hindrance to create a warning for a recursive call on the same execution
path as a recursive call, whether or not that second call is a tail call.

Rather than enumerate all the possible behaviors I could implement. I will
list just those that I'm considering (but feel free to suggest others that I
may have dismissed or not considered):

1. Suppress the warning only when a later call on that execution path has
already had a warning generated for it. Therefore suppressing the
warning for qsortapp but generating it for qsortacc.

2. Adding another option to the require_tail_recursion pragma to control
this. The option allows the developer to choose between the current
behaviour (always suppressing these warnings) and option 1
(suppressing the warnings when another call later in that path had a
warning generated for it) and always generating such warnings. If so
what shooed the default setting be and what should it be called?
Keeping in mind that if the user wants to guarantee constant stack
space (not a real guarantee because of higher order / module calls)
then they need this always turned on.

Thanks.
--
Paul Bone
Loading...