mthom / scryer-prolog

A modern Prolog implementation written mostly in Rust.
BSD 3-Clause "New" or "Revised" License
2.08k stars 125 forks source link

Too many indexing instructions #776

Open ghost opened 3 years ago

ghost commented 3 years ago

Using this code, inspired from the WAM book (page 81):

:- use_module(library(diag)).
:- use_module(library(format)).
:- use_module(library(lists)).

call_(call(X)) :- call_(X).
call_(repeat).
call_(repeat) :- call_(repeat).
call_(true).

code :-
    wam_instructions(call_/1, Is),
    maplist(portray_clause, Is).

call__(call(X)) :- call__(X).
call__(a).
call__(a) :- call__(a).
call__(true).

code_ :-
    wam_instructions(call__/1, Is),
    maplist(portray_clause, Is).

The instructions produced are different, while the code didn't change much:

$ scryer-prolog -g code -g halt choice
switch_on_term(1,4,1,0,5).
switch_on_constant(1,2).
try(7).
trust(9).
try_me_else(4).
get_structure(call,1,x(1)).
unify_variable(x(1)).
execute(call_,1).
retry_me_else(3).
get_constant(level(shallow),repeat,x(1)).
proceed.
retry_me_else(4).
get_constant(level(shallow),repeat,x(1)).
put_constant(level(shallow),repeat,x(1)).
execute(call_,1).
trust_me.
get_constant(level(shallow),true,x(1)).
proceed.
$

And:

$ scryer-prolog -g code_ -g halt choice
switch_on_term(1,6,1,0,7).
switch_on_constant(1,3).
try(9).
trust(11).
try(7).
trust(9).
try_me_else(4).
get_structure(call,1,x(1)).
unify_variable(x(1)).
execute(call__,1).
retry_me_else(3).
get_constant(level(shallow),a,x(1)).
proceed.
retry_me_else(4).
get_constant(level(shallow),a,x(1)).
put_constant(level(shallow),a,x(1)).
execute(call__,1).
trust_me.
get_constant(level(shallow),true,x(1)).
proceed.
$
  1. Why is there a difference?

With number, it's worse:

?- [user].
f(1).
f(1).

?- wam_instructions(f/1, Is), maplist(portray_clause, Is).
switch_on_term(1,8,1,0,0).
switch_on_constant(1,3).
try(7).
trust(9).
try(5).
trust(7).
try(3).
trust(5).
try_me_else(3).
get_constant(level(shallow),1,x(1)).
proceed.
trust_me.
get_constant(level(shallow),1,x(1)).
proceed.
...
  1. Is it bad code generation?
mthom commented 3 years ago

One of the improvements of the rebis-dev branch is that indices are printed in switch instructions:

?- code_.
switch_on_term(1,1,internal(1),fail,external(2)).
switch_on_constant([a:internal(1),a:internal(2),true:external(13)]).
try(6).
trust(9).
try(6).
trust(9).
try_me_else(4).
get_structure(call,1,x(1)).
unify_variable(x(1)).
execute(call__,1).
retry_me_else(3).
get_constant(level(shallow),a,x(1)).
proceed.
retry_me_else(4).
get_constant(level(shallow),a,x(1)).
put_constant(level(shallow),a,x(1)).
execute(call__,1).
trust_me(0).
get_constant(level(shallow),true,x(1)).
proceed.
   true.

The value a has two identical internal indices. It's not apparent from the print-out, but the reason it's indexed twice is because of the overlapping typed representations of a as an atom and as an Addr::Char.

Overlapping values are computed during code generation at https://github.com/mthom/scryer-prolog/blob/75908ab88fb5a28a1a33cde11aed2cebb2f96f68/src/indexing.rs#L938. See the Constant::Atom and Constant::Char cases and note the overlap.

I agree that they should point to the same internal index rather than stand-alones.

ghost commented 3 years ago

An atom/integer can have multiple representations in Rust but at unification it shouldn't matter. It could be solved that way. Or the Rust definition that represents an atom/integer could implement the equality (comparison, ...) test and it should solve it.

For the pair, (:)/2 is used, why not (-)/2?

ghost commented 3 years ago

The new switch_on_term/5 changed a lot, for code_/0:

master - switch_on_term(1,6,1,0,7).
rebis-dev - switch_on_term(1,1,internal(1),fail,external(2)).

On master, when the argument is a variable it's clear that the next instruction is try_me_else(4) but rebis-dev is making some assumptions. internal(1) seems simple but external(13) or external(2) isn't clear.

The external Prolog representation doesn't seems very compatible with the internal Rust representation.

mthom commented 3 years ago

The confusing thing about the new indexing representation is that indexing instructions are now stored in sub-vectors. A more accurate way to visualize the indexing code would be:

?- code_.
[switch_on_term(1,1,internal(1),fail,external(2)),
 switch_on_constant([a:internal(1),a:internal(2),true:external(13)]),
 [try(6),
 trust(9)],
 [try(6),
  trust(9)]
].
try_me_else(4).
get_structure(call,1,x(1)).

external(2) refers to get_structure(call,1,x(1)) and internal(1) and internal(2) refer to the two internal subvectors.

ghost commented 3 years ago

Couldn't it be represented in this way? It seems less confusing and it better reflects the Rust representation.