unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Guile CSE elimination of record accessor?
@ 2024-04-27 17:04 Simon Tournier
  2024-04-30 14:43 ` Andy Wingo
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Tournier @ 2024-04-27 17:04 UTC (permalink / raw)
  To: Guix Devel; +Cc: Ludovic Courtès

Hi,

In Guile module (ice-9 vlist), one reads:

--8<---------------cut here---------------start------------->8---
;; Asserting that something is a vlist is actually a win if your next
;; step is to call record accessors, because that causes CSE to
;; eliminate the type checks in those accessors.
;;
(define-inlinable (assert-vlist val)
  (unless (vlist? val)
    (throw 'wrong-type-arg
           #f
           "Not a vlist: ~S"
           (list val)
           (list val))))

[...]

(define (vlist-head vlist)
  "Return the head of VLIST."
  (assert-vlist vlist)
  (let ((base   (vlist-base vlist))
        (offset (vlist-offset vlist)))
    (block-ref (block-content base) offset)))
--8<---------------cut here---------------end--------------->8---

Other said, the argument ’vlist’ is “type-checked” with ’assert-vlist’
and thus that is exploited by Guile compiler, if I understand correctly
the comment.

The first question is: is it still correct?  Because this module had
been implemented before many Guile compiler improvements.


The second question, if the comment above is still valid, is: could we
also “win” for some record inside Guix source code?

Concretely, one example about the record <package>, there is some
procedures such that:

--8<---------------cut here---------------start------------->8---
(define* (package->manifest-entry package #:optional (output "out")
                                  #:key (parent (delay #f))
                                  (properties (default-properties package)))
  "Return a manifest entry for the OUTPUT of package PACKAGE."
  ;; For each dependency, keep a promise pointing to its "parent" entry.
  (letrec* ((deps  (map (match-lambda
                          ((label package)
                           (package->manifest-entry package
                                                    #:parent (delay entry)))
                          ((label package output)
                           (package->manifest-entry package output
                                                    #:parent (delay entry))))
                        (package-propagated-inputs package)))
            (entry (manifest-entry
                     (name (package-name package))
                     (version (package-version package))
                     (output output)
                     (item package)
                     (dependencies (delete-duplicates deps))
                     (search-paths
                      (package-transitive-native-search-paths package))
                     (parent parent)
                     (properties properties))))
    entry))
--8<---------------cut here---------------end--------------->8---

which fits the comment above: a record as argument and record accessor
call.

And that could also be applied to other records, I guess.


Any answers, explanations or references are very welcome. :-)

Cheers,
simon

PS: Raining day and weird pastime… diving into Guile source code. ;-)


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Guile CSE elimination of record accessor?
  2024-04-27 17:04 Guile CSE elimination of record accessor? Simon Tournier
@ 2024-04-30 14:43 ` Andy Wingo
  2024-05-03 18:08   ` Simon Tournier
  0 siblings, 1 reply; 3+ messages in thread
From: Andy Wingo @ 2024-04-30 14:43 UTC (permalink / raw)
  To: Simon Tournier; +Cc: Guix Devel, Ludovic Courtès

Hi :)

On Sat 27 Apr 2024 19:04, Simon Tournier <zimon.toutoune@gmail.com> writes:

> In Guile module (ice-9 vlist), one reads:
>
> ;; Asserting that something is a vlist is actually a win if your next
> ;; step is to call record accessors, because that causes CSE to
> ;; eliminate the type checks in those accessors.
> ;;
> (define-inlinable (assert-vlist val)
>   (unless (vlist? val)
>     (throw 'wrong-type-arg
>            #f
>            "Not a vlist: ~S"
>            (list val)
>            (list val))))
>
> [...]
>
> (define (vlist-head vlist)
>   "Return the head of VLIST."
>   (assert-vlist vlist)
>   (let ((base   (vlist-base vlist))
>         (offset (vlist-offset vlist)))
>     (block-ref (block-content base) offset)))
>
>
> Other said, the argument ’vlist’ is “type-checked” with ’assert-vlist’
> and thus that is exploited by Guile compiler, if I understand correctly
> the comment.
>
> The first question is: is it still correct?  Because this module had
> been implemented before many Guile compiler improvements.

No, the comment is incorrect.  The type check on whatever accessor is
called first (unspecified in scheme; probably we should just bite the
bullet and do predictable left-to-right semantics, as racket does) will
dominate the rest and eliminate those checks.  The assert-type is
unnecessary.

To see this, do ,optimize-cps at the repl, and count the number of
e.g. struct? checks with and without the assert-vlist.  There is only
one, either way.  (A type check is a heap-object? check, then struct?,
then get the vtable, then check against the global variable <vlist>.
All of these duplicates get eliminated.)

> PS: Raining day and weird pastime… diving into Guile source code. ;-)

:)

Cheers

Andy


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Guile CSE elimination of record accessor?
  2024-04-30 14:43 ` Andy Wingo
@ 2024-05-03 18:08   ` Simon Tournier
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Tournier @ 2024-05-03 18:08 UTC (permalink / raw)
  To: Andy Wingo; +Cc: Guix Devel, Ludovic Courtès

Hi Andy,

Thanks for the explanations.

On mar., 30 avril 2024 at 16:43, Andy Wingo <wingo@igalia.com> wrote:

>> The first question is: is it still correct?  Because this module had
>> been implemented before many Guile compiler improvements.
>
> No, the comment is incorrect.  The type check on whatever accessor is
> called first (unspecified in scheme; probably we should just bite the
> bullet and do predictable left-to-right semantics, as racket does) will
> dominate the rest and eliminate those checks.  The assert-type is
> unnecessary.

Good to know.

> To see this, do ,optimize-cps at the repl, and count the number of
> e.g. struct? checks with and without the assert-vlist.  There is only
> one, either way.

Hum, I am not sure to understand how to use ,optimize-cps at the repl.
Naively, I get:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(ice-9 vlist)
scheme@(guile-user)> ,optimize-cps (vlist-cons 'foo vlist-null)
L0:                                           ;              at <unknown>:102:14
  v0 := self
  L1(...)
L1:
  receive()
  v1 := current-module()                      ; mod While executing meta-command:
In procedure +: Wrong type argument in position 1: #f
--8<---------------cut here---------------end--------------->8---

Since ’,help compile’ reads,

    ,optimize-cps EXP         [,optx] - Run the CPS optimizer on a piece of code and print the result.

I assume that I do not feed with the correct expression EXP.  What would
be the invocation?


>                   (A type check is a heap-object? check, then struct?,
> then get the vtable, then check against the global variable <vlist>.
> All of these duplicates get eliminated.)

Ah yeah, it makes sense. :-)

Cheers,
simon


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-05-03 18:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-27 17:04 Guile CSE elimination of record accessor? Simon Tournier
2024-04-30 14:43 ` Andy Wingo
2024-05-03 18:08   ` Simon Tournier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).