Subject: [bug#53878] [PATCH 04/11] gnu: chez-and-racket-bootstrap: Add utilities for Chez machine types.

From: Philip McGrath <[email-masked]>
Date: Wed, 16 Feb 2022 17:54:58 -0500

Hi,

On 2/14/22 09:34, Liliana Marie Prikler wrote:
> Hi,
> 
> Am Sonntag, dem 13.02.2022 um 16:51 -0500 schrieb Philip McGrath:
>> [...]
>> +(define (chez-machine->upstream-restriction mach)
>> +  "Given a string MACH naming a Chez Scheme machine type, returns a
>> symbol
>> +naming a restriction on the upstream Chez Scheme implementation
>> compared to
>> +the Racket variant, or @code{#f} if no such restriction exists.  The
>> +restriction is reported for the architecture--OS pair, regardless of
>> whether
>> +MACH specifies a threaded or an unthreaded variant.
>> +
>> +Possible restrictions currently include:
>> +@itemize @bullet
>> +@item
>> +@code{'no-threads}: Support for native threads is not available
>> upstream.
>> +@item
>> +@code{'no-support}: The upstream release doesn't claim to support
>> this
>> +architecture--OS combination at all.
>> +@end itemize
>> +
>> +See @code{chez-machine->nix-system} for more details about
>> acceptable values
>> +for MACH."
>> +  (let ((mach (chez-machine->unthreaded mach)))
>> +    (cond
>> +     ((string-prefix? "arm64" mach)
>> +      'no-support)
>> +     ((string-prefix? "arm32" mach)
>> +      (if (string-suffix? "le" mach)
>> +          'no-threads
>> +          'no-support))
>> +     ((string-prefix? "ppc32" mach)
>> +      (if (string-suffix? "le" mach)
>> +          #f
>> +          'no-support))
>> +     (else
>> +      #f))))
> -> is a conversion operator, not an "accessor".

I thought of this as a conversion more than an accessor.

> "upstream-restriction" sounds rather negative, I'd rather have (chez-
> machine-features), which yields #f if the machine is unsupported and a
> (possibly empty) list of features otherwise, such as '(threads).
> 
> I'm also not quite sure what the point is behind using chez machines
> here.  Why not simply test the systems with the predicates we already
> have, i.e. target-arm64?, target-arm32?, target-linux?, target-ppc32?,
> ...

I agree that the name of this procedure should avoid sounding negative: 
'restriction' was the best I'd come up with so far. I think I like 
'chez-machine-features' somewhat better, but with a few reservations.

Using predicates like 'target-arm32?' makes some sense overall, and I'll 
try it. One subtly is that systems supported by neither upstream nor 
Racket are currently handled by `and=>` in clients.

Also, I guess it's odd to have this function operate on Chez Scheme 
machine types anyway, since a machine type specifies threaded or unthreaded.

I'm currently thinking something like:

--8<---------------cut here---------------start------------->8---
(define* (chez-upstream-features-for-system #:optional
                                             (system (or 
(%current-target-system)
 
(%current-system))))
   (cond
    ((not (nix-system->chez-machine system))
     #f)
    ((target-arm64? system)
     #f)
    ((target-arm32? system)
     (and (target-linux? system)
          '()))
    ((target-ppc32? system)
     (and (target-linux? system)
          '(threads)))
    (else
     '(threads))))
--8<---------------cut here---------------end--------------->8---

Alternatively, there could be an argument called something like 
"variant", where the default would be 'upstream or maybe #f, and 
supplying 'racket would always return '(threads). (Racket CS requires 
the threaded version of Chez Scheme, so the chez-scheme-for-racket has 
consistently added thread support immediately upon adding any new target.)

> 
> And as a minor pet peeve, you ought to spell out machine.
> 

Ok, will do.

> 
>> +(define* (nix-system->chez-machine #:optional (system (%current-
>> system))

This should have been (or (%current-target-system) (%current-system)).

>> +                                   #:key (threads? 'always))
>> +  "Return the Chez Scheme machine type corresponding to the Nix
>> system
>> +identifier SYSTEM, or @code{#f} if the translation of SYSTEM to a
>> Chez Scheme
>> +machine type is undefined.
>> +
>> +When THREADS? is @code{'always} (the default), the threaded variant
>> of the
>> +machine type will be returned: note that the package returned by
>> +@code{chez-scheme-for-system} will always support native threads.
>> When
>> +THREADS? is @code{#f}, the unthreaded machine type will be
>> returned.  If
>> +THREADS? is @code{'upstream} (the default), the threaded variant of
>> the
>> +machine type will be returned if and only if it is supported by
>> upstream Chez
>> +Scheme (see @code{chez-machine->upstream-restriction}).  If THREADS?
>> is any
>> +other value, an exception is raised."
> What's the point in having THREADS? 'always?  In any case, assuming
> chez-machine-features is to be exported, this can easily be checked --
> even if not, we can add the check internally by writing
>    #:key (threads? (chez-supports-threads? system))
> 
>> +  (let* ((hyphen (string-index system #\-))
>> +         (nix-arch (substring system 0 hyphen))
>> +         (nix-os (substring system (+ 1 hyphen)))
>> +         (chez-arch (assoc-ref %nix-arch-to-chez-alist nix-arch))
>> +         (chez-os (assoc-ref %nix-os-to-chez-alist nix-os))
>> +         (mach (and chez-arch chez-os (string-append chez-arch chez-
>> os))))
> This series of let-bindings should probably be done in a separate
> function called nix-system->chez-machine.

On the one hand, the result of 'chez-scheme-for-system' will always 
support threads, so being threaded by default is convenient. On the 
other hand, it looks like I've reduced the use of this function (by 
searching for files rather than coding in the machine type directory) 
enough that it doesn't have a big impact either way. It will be more 
important when we support cross-compilation, and I think we should keep 
these functions internal and defer as much as possible of their API 
design until then.

For now, I think I'll say that it's unspecified whether the result is a 
threaded or unthreaded machine type and add 
`chez-machine->{un,}threaded` as needed.

-Philip