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

[Segher Boessenkool]

On Tue, Oct 10, 2017 at 01:23:15PM +1100, Alexey Kardashevskiy wrote:
> On 10/10/17 04:28, Segher Boessenkool wrote:
> >>>> 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?

The name lookup is *the* slowest part of the engine, you do not want to
make it slower for no good reason.

The word fields are not a fixed-length struct, so you cannot describe
that with a STRUCT (and there already are NAME>LINK LINK> NAME> words,
and no code outside of the engine has any business accessing flags,
count, chars directly).

It's fine to document this of course.

If other code wants to do unusual things to wordlists we really should
abstract that to some other words, not "manually" manipulate memory
(which you will still do with the "struct" definitions).  Yup I'm to
blame for the first occurrence of this here ;-)

> 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'?

It's not really good either ;-)

: for-all-words ( wid xt -- ) \ xt has sig ( lfa -- )
   dup >r cell+ @ BEGIN dup WHILE dup  r@ execute  @ REPEAT  r> drop ;
: fdt-copy-properties ( phandle -- )
   dup encode-int s" phandle" fdt-prop
   node>properties @ ['] fdt-copy-property for-all-words ;

Maybe it's nicer to use the nfa... dunno.

> > 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.

\ link>name ( lfa -- nfa )
\ name> ( nfa -- xt )
\ link> ( lfa -- xt )

The name field is the flags, the count, the chars, and padding to cell size.
The nfa is the address of the name field, so the address of the flags.

> > 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

Nope, a wordlist is something else.  I shouldn't have said list: it's
just a bunch of xt's one after another in memory.  Like an array.

> 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?

A colon def like:
: over  2dup drop ;  \ a silly way to implement over of course :-)
is laid out in memory like (assuming cell size 8):

link                    \ to the previous word in this word list
00 04 6f 76 65 72 00 00 \ flags=0, "over" as counted string, padding
' DOCOL                 \ the address of docol; "nest"
' 2dup
' drop
' EXIT                  \ or SEMIS or whatever is the end of a colon
                        \ word these days; "unnest"

> > 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
> ;

Because that won't even fit more than two definitions on a screen.  Rigid
identation practices like that also make it impossible to use spacing to
indicate where phrases end.  And, importantly, the only reason you would
want indents is if you are nesting too deep (i.e. at all).

Most definitions should fit on a single line!

> >>>> (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 ;)

If a name makes clear what the word does, it is a good name, especially
if it is a nice short name as well.  If it is hard to make a nice short
name that describes exactly what the word does, chances are the word
does too much.

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

I think "copy" is better than "add"?


Segher