[OpenBIOS] [PATCH 1/3] bootstrap.fs: add pseudo r-stack implementation for interpret mode

Mark Cave-Ayland mark.cave-ayland at ilande.co.uk
Sun Jul 17 22:42:47 CEST 2016


On 17/07/16 20:25, Segher Boessenkool wrote:

> On Sun, Jul 17, 2016 at 08:05:33PM +0100, Mark Cave-Ayland wrote:
>>>> +\
>>>> +\ Pseudo r-stack implementation for interpret mode
>>>> +\
>>>> +
>>>> +variable prstack 20 cells allot
>>>> +variable #prstack 0 #prstack !
>>>
>>> 20 hex or 20 decimal?  If it isn't clear, put hex or decimal before this?
>>
>> The default in OpenBIOS is hex, so the intended value is 32 decimal.
>> Skimming through the existing files, it looks like the conventional way
>> to do "h# 20" so I can do that on a v2.
> 
> If the convention is to have hex everywhere, then your existing code is
> fine.  h# 20 is clear as well.

Great, thanks - I'll go with that for now.

>>> This allocates 21 cells and isn't portable, btw ("variable" allocate one
>>> cell already, not necessarily contiguous with the "allot" allocated cells).
>>
>> Looking at the existing code, the most common way to reserve memory in
>> the dictionary is with allot, e.g.
>>
>> variable prstack here h# 20 cells allot prstack !
>>
>> Does that look reasonable?
> 
> No, that means something entirely different (extra indirection).
> 
> create   version1 20 cells allot
> variable version2 20 cells allot
> 
> version1 is portable and allocates 20 cells.
> version2 is not portable, and allocates 21 cells.

Looks like the issue with create is that it triggers a bug in the Forth
bootstrap by the C kernel. If I move the r-stack functions into a
separate file which is compiled by the Forth engine then it works fine
so I'm tempted to do that for now (plus it means the file can be
removed/changed for one or all architectures if required although I
haven't seen a regression across all my tests with the patch applied).

I'll make the change here and repost the entire patchset as a v2.

>>>> +: >r state @ if ['] >r , exit then r> swap prstack-push >r ; immediate
>>>> +: r> state @ if ['] r> , exit then r> prstack-pop swap >r ; immediate
>>>> +: r@ state @ if ['] r@ , exit then r> prstack-pop dup prstack-push swap >r ; immediate
>>>
>>> ['] >r ,   is very non-portable as well, "postpone >r" is much more modern.
>>
>> The version above is what is used throughout the OpenBIOS codebase as
>> opposed to postpone which is why I went with it. I've just tried using
>> postpone in OpenBIOS and it causes things to break, so I suspect that
>> either it does something strange to the r-stack itself or perhaps it is
>> another word that needs fixes for immediate mode.
> 
> Huh, wow.  The whole point of POSTPONE is that you can use it on words
> that are either IMMEDIATE or not; i.e. it does the jobs of both [COMPILE]
> and COMPILE .  Strange that POSTPONE does not work for you.

I tried the use of postpone again in the separate file, but that still
doesn't work so there must be something else wrong the implementation in
OpenBIOS. Hmmm.


ATB,

Mark.




More information about the OpenBIOS mailing list