Skip to content

Fix MAKE-LOAD-FORM data-flow ordering in the file compiler#741

Closed
blakemcbride wants to merge 1 commit intoarmedbear:masterfrom
blakemcbride:make-load-form
Closed

Fix MAKE-LOAD-FORM data-flow ordering in the file compiler#741
blakemcbride wants to merge 1 commit intoarmedbear:masterfrom
blakemcbride:make-load-form

Conversation

@blakemcbride
Copy link
Copy Markdown

Fix MAKE-LOAD-FORM data-flow ordering in the file compiler

Summary

  • Fixes all 12 failing MAKE-LOAD-FORM.ORDER.* tests (ORDER.3–14) in
    the ANSI test suite: compiled-test failures drop from 64 to 52
    with no regressions.
  • The file compiler now emits MAKE-LOAD-FORM creation and
    initialization forms to the fasl in data-flow dependency order,
    with init forms run ASAP per CLHS.
  • Bumps *FASL-VERSION* from 43 to 44 (fasl layout changed).

Motivation

CLHS requires, for every literal instance handled by
MAKE-LOAD-FORM:

  • the creation form runs after creation of every object it
    references (transitive closure);
  • the init form runs after all objects it references are created,
    and as soon as its data-flow dependencies are satisfied.

Previously, ABCL emitted the creation + init forms inline at the
point where the instance was referenced, expanded via
get-instance-form as (let ((inst <creation>)) <init> inst). This
could not express cross-object dependencies and failed every
MAKE-LOAD-FORM.ORDER.* test that involved more than one instance
with dependencies between them.

Approach

Each fasl now carries a per-file instance vector
sys::*fasl-instances* indexed by integer. For every literal
instance referenced in the compile unit, the compiler emits (in
dependency order, before the top-level form that references it):

(setf (svref sys::*fasl-instances* N) <creation-form>)
<initialization-form>

and the original inline reference to the instance is rewritten to
#.(svref sys::*fasl-instances* N). The vector is allocated once in
the fasl prologue from the final instance count.

A two-phase dependency walker drives the emission:

  1. %fasl-ensure-created(X) — recursively ensures creation of every
    instance embedded in X's creation form, then emits X's
    creation form. A re-entry while X is mid-creation is a real
    circular creation dependency and signals an error (CLHS: undefined
    behavior).
  2. %fasl-ensure-initialized(X) — ensures X is created, then
    recursively ensures initialization of every instance embedded in
    X's init form, then emits X's init form. An in-progress init
    cycle is tolerated (CLHS leaves ordering within an init cycle
    unspecified).

To honor the ASAP rule (tests .6, .10.12),
%fasl-ensure-created eagerly initializes X immediately after
emitting its creation form iff %fasl-init-deps-ready-p reports
that every object referenced by X's init form (ignoring X itself,
since MAKE-LOAD-FORM methods commonly embed ',x in their init
form) is already initialized. Otherwise init-X is emitted later,
either from the top-level walk that directly references X or from
another object's init-form walk.

Circularity detection

dump-instance now writes #.(svref sys::*fasl-instances* N)
instead of the legacy inline form. The existing *circularity*
walker (df-check-object) had to be taught to walk the same cons
that dump-instance ultimately writes, so a single reference cons
per instance is cached in *fasl-instance-refs* and reused by both
sites.

A new dynamic variable *fasl-emitting-to-fasl-stream* gates the new
behavior: when unbound (e.g. dumping constants into a class file via
SERIALIZE-OBJECT), dump-instance / df-check-instance fall back
to the legacy get-instance-form expansion.

Files changed

  • src/org/armedbear/lisp/Load.java
    • Bump *FASL-VERSION* from 43 to 44.
    • Add the *FASL-INSTANCES* special; bindSpecial it per fasl in
      init_fasl.execute.
  • src/org/armedbear/lisp/dump-form.lisp
    • Add per-fasl instance tables and the two-phase dep walk
      (%fasl-ensure-created, %fasl-ensure-initialized,
      %fasl-walk-for-deps, %fasl-init-deps-ready-p,
      %fasl-write-raw-form, %fasl-emit-toplevel-form).
    • Teach dump-instance and df-check-instance to use the cached
      ref cons when emitting to a fasl stream; retain the legacy
      get-instance-form path for non-fasl contexts.
  • src/org/armedbear/lisp/compile-file.lisp
    • Bind the new instance-table specials in compile-from-stream.
    • Emit (setq sys::*fasl-instances* (make-array N)) in the fasl
      prologue when any instance was registered.
    • Route top-level forms through %fasl-emit-toplevel-form.

Test plan

  • Smoke test mirroring MAKE-LOAD-FORM.ORDER.4 runs in the
    expected order:
    *ORDER* = (:CREATING B :CREATING A :INITIALIZING A :INITIALIZING B) (eql (ob-init-dep *b*) *a*) = T
  • ant test.ansi.compiled: 64 → 52 failures / 21930 tests.
  • All of MAKE-LOAD-FORM.ORDER.3 through .14 now pass.
  • No new failures vs. baseline on this branch.

Compatibility

Fasl layout has changed (new prologue setq of
sys::*fasl-instances*, new per-reference (svref ...) form), hence
the *FASL-VERSION* bump. Older .abcl fasls are rejected by the
version check in Load.java and must be recompiled.

@blakemcbride
Copy link
Copy Markdown
Author

Duplicated in pull request 743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant