[SLOF] [PATCH slof v3 5/5] fdt: Pass the resulting device tree to QEMU

[Alexey Kardashevskiy]

On 10/10/17 04:28, Segher Boessenkool wrote:
> On Mon, Oct 09, 2017 at 01:06:08PM +1100, Alexey Kardashevskiy wrote:
>> This is the mail I wanted you to comment on. Thanks!
> 
> Ah ok, thanks.
> 
>>>> Well.  The wordlists are themselves a linked list; offset 0 holds the
>>>> address of the next wordlist, and offset 1 cell holds the address of
>>>> the first name record.  Like:
>>>>
>>>> STRUCT
>>>>   cell FIELD wid>next
>>>>   cell FIELD wid>names \ the head of the list of name records
>>>> END-STRUCT
>>>>
>>>> STRUCT
>>>>   cell FIELD name>next
>>>>   char FIELD name>flags
>>>>   char FIELD name>count
>>>>   0    FIELD name>chars
>>>> END-STRUCT
>>>
>>>
>>> These do not exist in the existing SLOF code, you just typed them here, right?
> 
> Yes, this is just a description of what the core code (engine.in) does.
> You do not actually want these words, certainly not in node.fs .

Even declaring these is bad? Elsewhere you suggested:

: fdt-properties ( phandle -- )
   dup encode-int s" phandle" fdt-prop
   node>properties @ cell+ @ BEGIN dup WHILE dup fdt-property @ REPEAT drop ;

How bad is replacing an absolutely magic 'cell+' with a kinda documented
'wid>names'?


> 
> For example, "whatever>next" is just a no-op: writing it as no code at
> all is faster, this is quite hot code.

Sure, wid>next and name>next are useless in the code but they still serve a
documentation purpose, and other struct fields are no no-op.


> 
>>>> : link>  link>name name> ;
>>>
>>> Ahhh. I was searching for 'LINK_X3E' but it is:
>>> col(LINK> LINK>NAME NAME>)
>>>
>>> Ok. Another confusion was that 'NAME' here does not point to a string but
>>> to 'name>flags'. Ok.
> 
> Yeah, engine.in is mostly like Forth syntax (ignoring all the immediate
> fields, and the words headers ("col" etc.)
> 
>>> btw what does 'lfa' stand for in stack comments?
> 
> lfa is "link field address".  nfa is "name field address".  xt is
> "execution token".  SLOF uses a very traditional dictionary structure.
> Each word is laid out like this:
> 
> lfa:	cell, a pointer to the previous word in this list.
> nfa:	at least one cell; a flags field, and the name of the word as
> 	a counted string, and padding to the next word boundary.

So "nfa" is name>next + name>flags + name>count + name>chars + padding. Ok.


> xt:	the "body" of the word, starting with the code field, and then
> 	for e.g. colon words a list of (mostly) xts, etc.


So it is a wordlist where the very first word has the node's name and its
xt returns the raw date (pointer + len), and other words do some other
stuff (.print-xxxx?), is that correct? Where is that very first word
compiled to the xt which returns the raw data?




> 
>>> So, after 'link>', the stack is:
>>> 			( properties-names-next end-of-name )
> 
> : link> ( lfa -- xt )
> 
>>> '>name' simply goes backwards to the beginning of the name, right (if I am
>>> reading the beginning of slof/fs/base.fs correctly)?
> 
> Yes, but it guesses: it searches back to a byte that could be the
> correct length byte.  It is also slow.  You do not want to use it
> (unless you have to, to get a name when only knowing the xt: it's
> really only for debugging things).
> 
>>> btw '.property' still does search, what is that - room for a little
>>> improvement or I am missing something again?
> 
> Probably, yes.  Seems it could just use   link>name   instead of
> link> >name   .
> Well, it is   link> dup >name name>string   so it could do
> link>name dup name> swap name>string   or something like that.

Aha! :)


> 
>>>> : fdt-property ( link -- )
>>>>    dup link> execute rot link>name name>string fdt-prop ;
>>>
>>> Without stack comments, I'll forget this in 2 months maximum.
> 
> (There is a stack comment :-P )
> 
> I don't write "what is on the stack" after every word, certainly not
> in short phrases, where it should be obvious.
> 
> I suspect your problem reading this is that you do not know the stack
> signatures of link> etc. by heart.


Exactly :(


>>>> : fdt-properties ( phandle -- )
>>>>    dup encode-int s" phandle" fdt-prop
>>>>    node>properties @ cell+ @ BEGIN dup WHILE dup fdt-property @ REPEAT drop ;
>>>
>>>
>>> With this formatting and missing stack comments I will stop understanding
>>> it in a week :(
> 
> A bit better factored then?
> 
> : (fdt-properties) ( lfa -- )
>    BEGIN dup WHILE dup fdt-property @ REPEAT drop ;
> : fdt-properties ( phandle -- )
>    dup encode-int s" phandle" fdt-prop
>    node>properties @ cell+ @ (fdt-properties) ;

May be... I'll do this but I am still struggling with a single line loops.
Why exactly is this style bad?

: fdt-add-properties ( phandle -- )
    dup encode-int s" phandle" fdt-prop

    node>properties @ wid>names @
    BEGIN
        dup
    WHILE
        dup fdt-add-property
        @
    REPEAT
    drop
;


> 
>>>> (and maybe better names?  Something that makes clear it is copying
>>>> properties to the flat tree).
>>>
>>> It does not work like this, if you do not like the name - do not just say
>>> it, suggest a better one ;)
> 
> It certainly does work like that!  Coming up with good names is the
> hardest part of programming (and a very important part).

You have a better idea what is a good forth name ;)

> Maybe just names like "make-fdt-properties"?


I have now fdt-add-properties and fdt-add-property, too bad?


> 
> Cheers, HTH,
> 
> 
> Segher
> 


-- 
Alexey