packer & compiler test
This commit is contained in:
parent
3fbcb30dff
commit
d7a26473c8
@ -0,0 +1,127 @@
|
|||||||
|
# Ignored Call Results in Executable Lowering Agenda
|
||||||
|
|
||||||
|
## Status
|
||||||
|
|
||||||
|
Open
|
||||||
|
|
||||||
|
## Domain Owner
|
||||||
|
|
||||||
|
`docs/compiler/pbs`
|
||||||
|
|
||||||
|
Este tema pertence ao domínio PBS do compiler porque afeta:
|
||||||
|
|
||||||
|
- semântica prática de `expression statement`;
|
||||||
|
- lowering executável para `IRBackend`/`IRVM`;
|
||||||
|
- ergonomia de uso de SDKs e hosts que retornam status;
|
||||||
|
- compatibilidade entre o que o código-fonte parece permitir e o que o pipeline realmente aceita.
|
||||||
|
|
||||||
|
## Problema
|
||||||
|
|
||||||
|
Hoje uma chamada com retorno usada como statement isolado pode deixar valor sobrando na stack no lowering executável.
|
||||||
|
|
||||||
|
Exemplo real:
|
||||||
|
|
||||||
|
```pbs
|
||||||
|
Gfx.set_sprite(...);
|
||||||
|
```
|
||||||
|
|
||||||
|
No estado atual, isso compila no frontend, mas pode falhar no pipeline backend com erro de validação de stack no `RET` da função, porque o resultado da call não foi consumido explicitamente.
|
||||||
|
|
||||||
|
Isso cria uma inconsistência operacional:
|
||||||
|
|
||||||
|
1. o código parece válido para o autor;
|
||||||
|
2. o frontend aceita a forma;
|
||||||
|
3. o backend exige, na prática, um consumo manual do retorno, por exemplo:
|
||||||
|
|
||||||
|
```pbs
|
||||||
|
let sprite_status = Gfx.set_sprite(...);
|
||||||
|
```
|
||||||
|
|
||||||
|
## Contexto
|
||||||
|
|
||||||
|
O problema apareceu ao migrar o consumo de sprite em PBS para o contrato novo de `Gfx.set_sprite`, que retorna um status inteiro.
|
||||||
|
|
||||||
|
Os pontos observados no código atual são:
|
||||||
|
|
||||||
|
- `ExpressionStatement` faz lowering apenas da expressão;
|
||||||
|
- o lowering de `CallExpr` emite `CALL_HOST`, `CALL_FUNC` ou `CALL_INTRINSIC` com `retSlots`;
|
||||||
|
- não há descarte automático do valor quando a expressão é usada apenas como statement;
|
||||||
|
- o validador de `IRVM` exige que funções `void` retornem com stack height `0`.
|
||||||
|
|
||||||
|
Na prática, isso transforma "ignorar retorno" em comportamento parcialmente suportado pela linguagem, mas não suportado pelo pipeline executável.
|
||||||
|
|
||||||
|
## Opções
|
||||||
|
|
||||||
|
### Opção A
|
||||||
|
|
||||||
|
Manter o comportamento atual e exigir consumo explícito de todo retorno em PBS.
|
||||||
|
|
||||||
|
### Opção B
|
||||||
|
|
||||||
|
Permitir que `expression statements` descartem automaticamente o resultado quando a expressão produzir valor.
|
||||||
|
|
||||||
|
### Opção C
|
||||||
|
|
||||||
|
Permitir descarte automático apenas para um subconjunto de calls, como hosts/SDKs anotados como status descartável.
|
||||||
|
|
||||||
|
## Tradeoffs
|
||||||
|
|
||||||
|
### Opção A
|
||||||
|
|
||||||
|
- Prós:
|
||||||
|
- regra simples no backend;
|
||||||
|
- evita descarte implícito sem intenção.
|
||||||
|
- Contras:
|
||||||
|
- ergonomia ruim para APIs baseadas em status;
|
||||||
|
- surpresa para autores, porque `foo();` parece natural mas falha mais tarde;
|
||||||
|
- expõe detalhe de stack do backend ao código-fonte.
|
||||||
|
|
||||||
|
### Opção B
|
||||||
|
|
||||||
|
- Prós:
|
||||||
|
- comportamento esperado para statement de expressão;
|
||||||
|
- reduz ruído em código de jogo e SDK;
|
||||||
|
- alinha parsing, semântica prática e lowering executável.
|
||||||
|
- Contras:
|
||||||
|
- introduz descarte implícito;
|
||||||
|
- exige regra clara para saber quando emitir `POP`.
|
||||||
|
|
||||||
|
### Opção C
|
||||||
|
|
||||||
|
- Prós:
|
||||||
|
- mais controle semântico;
|
||||||
|
- evita descarte implícito amplo.
|
||||||
|
- Contras:
|
||||||
|
- adiciona complexidade de contrato e metadata;
|
||||||
|
- mistura política de produto/API dentro do lowering;
|
||||||
|
- não resolve a expectativa geral sobre `expression statement`.
|
||||||
|
|
||||||
|
## Recomendação
|
||||||
|
|
||||||
|
Seguir com a **Opção B**.
|
||||||
|
|
||||||
|
Direção recomendada:
|
||||||
|
|
||||||
|
1. `expression statement` deve ser permitido mesmo quando a expressão produzir valor;
|
||||||
|
2. o lowering executável deve emitir descarte explícito do resultado não usado;
|
||||||
|
3. a regra deve ser geral para expressões com resultado materializado em stack, não especial para `Gfx.set_sprite`;
|
||||||
|
4. testes de regressão devem cobrir host calls, callable calls e intrinsics com retorno ignorado.
|
||||||
|
|
||||||
|
Essa direção preserva a ergonomia esperada da linguagem e remove um vazamento indevido do modelo de stack do backend para o código PBS.
|
||||||
|
|
||||||
|
## Perguntas em Aberto
|
||||||
|
|
||||||
|
1. O descarte automático deve valer para qualquer `expression statement` com valor ou apenas para formas lowerables em v1?
|
||||||
|
2. O frontend semântico deve emitir algum warning opcional para retorno ignorado, ou isso fica fora do escopo atual?
|
||||||
|
3. O descarte deve acontecer somente no lowering executável ou também virar regra explícita de spec para statements?
|
||||||
|
4. Existe algum caso em que o descarte automático possa mascarar bug real de usuário que hoje seria detectado mais cedo?
|
||||||
|
|
||||||
|
## Próximo Passo Sugerido
|
||||||
|
|
||||||
|
Converter esta agenda em uma `decision` do domínio `compiler/pbs` fechando a política para `expression statement` com resultado ignorado.
|
||||||
|
|
||||||
|
Depois disso, abrir um `pull-request/plan` curto para:
|
||||||
|
|
||||||
|
1. ajustar o lowering de `ExpressionStatement`;
|
||||||
|
2. adicionar fixtures e testes de regressão no frontend/backend pipeline;
|
||||||
|
3. propagar a regra para specs relevantes de statements e lowering.
|
||||||
@ -11,6 +11,7 @@ Closed agendas are moved to `docs/pbs/agendas/archive`.
|
|||||||
3. `18.2. Backend Workshop 2 - LowerToIRVM and IRVM Contract.md`
|
3. `18.2. Backend Workshop 2 - LowerToIRVM and IRVM Contract.md`
|
||||||
4. `18.3. Backend Workshop 3 - Bytecode Marshaling and Runtime Conformance.md`
|
4. `18.3. Backend Workshop 3 - Bytecode Marshaling and Runtime Conformance.md`
|
||||||
5. `18.4. Asset References in Game Code - Names vs Compile-Time Lowering Agenda.md`
|
5. `18.4. Asset References in Game Code - Names vs Compile-Time Lowering Agenda.md`
|
||||||
|
6. `18.5. Ignored Call Results in Executable Lowering Agenda.md`
|
||||||
|
|
||||||
## Purpose
|
## Purpose
|
||||||
|
|
||||||
|
|||||||
@ -7,6 +7,7 @@ This directory contains active packer discussion agendas.
|
|||||||
1. [`Tilemap and Metatile Runtime Binary Layout Agenda.md`](./Tilemap%20and%20Metatile%20Runtime%20Binary%20Layout%20Agenda.md)
|
1. [`Tilemap and Metatile Runtime Binary Layout Agenda.md`](./Tilemap%20and%20Metatile%20Runtime%20Binary%20Layout%20Agenda.md)
|
||||||
2. [`Pack Wizard Studio Decision Propagation Agenda.md`](./Pack%20Wizard%20Studio%20Decision%20Propagation%20Agenda.md)
|
2. [`Pack Wizard Studio Decision Propagation Agenda.md`](./Pack%20Wizard%20Studio%20Decision%20Propagation%20Agenda.md)
|
||||||
3. [`Tile Bank Packing Materialization Agenda.md`](./Tile%20Bank%20Packing%20Materialization%20Agenda.md)
|
3. [`Tile Bank Packing Materialization Agenda.md`](./Tile%20Bank%20Packing%20Materialization%20Agenda.md)
|
||||||
|
4. [`Variable Tile Bank Palette Serialization Agenda.md`](./Variable%20Tile%20Bank%20Palette%20Serialization%20Agenda.md)
|
||||||
|
|
||||||
The first packer agenda wave was consolidated into normative specs under [`../specs/`](../specs/) and didactic material under [`../learn/`](../learn/).
|
The first packer agenda wave was consolidated into normative specs under [`../specs/`](../specs/) and didactic material under [`../learn/`](../learn/).
|
||||||
|
|
||||||
|
|||||||
@ -0,0 +1,142 @@
|
|||||||
|
# Variable Tile Bank Palette Serialization Agenda
|
||||||
|
|
||||||
|
## Status
|
||||||
|
|
||||||
|
Active
|
||||||
|
|
||||||
|
## Domain Owner
|
||||||
|
|
||||||
|
- `docs/packer`
|
||||||
|
|
||||||
|
Cross-domain dependency:
|
||||||
|
|
||||||
|
- `../runtime`
|
||||||
|
|
||||||
|
## Purpose
|
||||||
|
|
||||||
|
Decide whether `tile bank` payloads should keep serializing a fixed block of `64` palettes in v1, or evolve to serialize only the palettes actually authored by the asset.
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
The current `tile bank` contract is aligned between `packer` and `runtime`:
|
||||||
|
|
||||||
|
- pixels are serialized as packed `u4`;
|
||||||
|
- palettes are serialized as `RGB565 u16`;
|
||||||
|
- `palette_count = 64`;
|
||||||
|
- payload size is derived from `ceil(width * height / 2) + (64 * 16 * 2)`.
|
||||||
|
|
||||||
|
This means a `tile bank` with `1` authored palette still serializes `64` palette slots. Unused slots are currently zero-filled, which is valid for the runtime, but wastes cartridge space.
|
||||||
|
|
||||||
|
The packer now exposes:
|
||||||
|
|
||||||
|
- `metadata.palette_count = 64`
|
||||||
|
- `metadata.palette_authored = <actual authored count>`
|
||||||
|
|
||||||
|
This makes the waste explicit, but does not remove it.
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
The runtime-facing v1 contract favors fixed-size decode simplicity over cartridge efficiency.
|
||||||
|
|
||||||
|
We need to decide whether:
|
||||||
|
|
||||||
|
1. fixed `64` palette slots remain the normative contract for `tile bank`;
|
||||||
|
2. `palette_count` becomes variable and payload size shrinks to the authored set;
|
||||||
|
3. another staged strategy is better.
|
||||||
|
|
||||||
|
## Options
|
||||||
|
|
||||||
|
### Option A: Keep Fixed 64-Palette Serialization
|
||||||
|
|
||||||
|
Keep the current contract unchanged:
|
||||||
|
|
||||||
|
- `palette_count` remains `64`;
|
||||||
|
- every `tile bank` serializes `64 * 16 * 2 = 2048` bytes of palettes;
|
||||||
|
- authored palettes populate their indexed slots;
|
||||||
|
- unused slots are zero-filled.
|
||||||
|
|
||||||
|
### Option B: Move to Variable Palette Serialization
|
||||||
|
|
||||||
|
Evolve the contract so `tile bank` serializes only authored palettes:
|
||||||
|
|
||||||
|
- `palette_count` becomes the authored count;
|
||||||
|
- payload size becomes `ceil(width * height / 2) + (palette_count * 16 * 2)`;
|
||||||
|
- runtime validates and decodes variable palette blocks.
|
||||||
|
|
||||||
|
### Option C: Keep V1 Fixed, Design Variable Serialization for V2
|
||||||
|
|
||||||
|
Preserve the current runtime-facing v1 contract now, but explicitly open a v2 format/contract:
|
||||||
|
|
||||||
|
- v1 stays fixed at `64`;
|
||||||
|
- v2 introduces variable palette serialization;
|
||||||
|
- packer and runtime keep clear compatibility boundaries.
|
||||||
|
|
||||||
|
## Tradeoffs
|
||||||
|
|
||||||
|
### Option A
|
||||||
|
|
||||||
|
Pros:
|
||||||
|
|
||||||
|
- no further runtime work;
|
||||||
|
- simplest decode path;
|
||||||
|
- current specs and tests stay valid.
|
||||||
|
|
||||||
|
Cons:
|
||||||
|
|
||||||
|
- every `tile bank` pays the full 2 KB palette cost;
|
||||||
|
- cartridge size is inflated for small authored palette sets;
|
||||||
|
- the metadata now exposes a mismatch between authored and serialized counts by design.
|
||||||
|
|
||||||
|
### Option B
|
||||||
|
|
||||||
|
Pros:
|
||||||
|
|
||||||
|
- reduces cartridge size;
|
||||||
|
- makes `palette_count` semantically tighter;
|
||||||
|
- removes fixed empty palette padding from the payload.
|
||||||
|
|
||||||
|
Cons:
|
||||||
|
|
||||||
|
- requires coordinated changes in `packer`, `runtime`, specs, and tests;
|
||||||
|
- changes the `size` / `decoded_size` formulas;
|
||||||
|
- raises compatibility and migration questions for existing assets and cartridges.
|
||||||
|
|
||||||
|
### Option C
|
||||||
|
|
||||||
|
Pros:
|
||||||
|
|
||||||
|
- keeps current work stable;
|
||||||
|
- makes room for a cleaner future contract;
|
||||||
|
- avoids mixing optimization work into the current tile-bank rollout.
|
||||||
|
|
||||||
|
Cons:
|
||||||
|
|
||||||
|
- known waste remains in v1;
|
||||||
|
- adds future format/version management work.
|
||||||
|
|
||||||
|
## Recommendation
|
||||||
|
|
||||||
|
Recommendation for now: `Option C`.
|
||||||
|
|
||||||
|
Reasoning:
|
||||||
|
|
||||||
|
- the current fixed-size contract is already implemented and aligned;
|
||||||
|
- the waste is real, but it is not a correctness bug;
|
||||||
|
- changing it now would reopen a cross-domain runtime contract that was just stabilized;
|
||||||
|
- if we want variable palette serialization, it should be introduced deliberately as a versioned follow-up, not as an incidental tweak.
|
||||||
|
|
||||||
|
## Open Questions
|
||||||
|
|
||||||
|
1. Should variable palette serialization be introduced as a new `tile bank` payload version, or as an incompatible change to the current one?
|
||||||
|
2. If `palette_count` becomes variable, should runtime still materialize a 64-slot bank in memory, or truly shrink the resident representation too?
|
||||||
|
3. Should palette slot identity remain sparse by authored `index`, or should serialization canonicalize into a dense runtime block?
|
||||||
|
4. Which domain owns the compatibility story for existing cartridges: `packer`, `runtime`, or `shipper`?
|
||||||
|
|
||||||
|
## Expected Follow-up
|
||||||
|
|
||||||
|
If the team wants to eliminate the fixed 64-palette padding:
|
||||||
|
|
||||||
|
1. create a corresponding runtime agenda/decision;
|
||||||
|
2. define the versioning and compatibility story;
|
||||||
|
3. update `packer` and `runtime` specs together;
|
||||||
|
4. then plan code changes in both domains.
|
||||||
@ -28,7 +28,7 @@ declare host LowGfx {
|
|||||||
[Host(module = "gfx", name = "set_sprite", version = 1)]
|
[Host(module = "gfx", name = "set_sprite", version = 1)]
|
||||||
[Capability(name = "gfx")]
|
[Capability(name = "gfx")]
|
||||||
fn set_sprite(
|
fn set_sprite(
|
||||||
asset_name: str,
|
bank_id: int,
|
||||||
index: int,
|
index: int,
|
||||||
x: int,
|
x: int,
|
||||||
y: int,
|
y: int,
|
||||||
@ -82,7 +82,7 @@ declare service Gfx
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn set_sprite(
|
fn set_sprite(
|
||||||
asset_name: str,
|
bank_id: int,
|
||||||
index: int,
|
index: int,
|
||||||
x: int,
|
x: int,
|
||||||
y: int,
|
y: int,
|
||||||
@ -94,7 +94,7 @@ declare service Gfx
|
|||||||
priority: int
|
priority: int
|
||||||
) -> int
|
) -> int
|
||||||
{
|
{
|
||||||
return LowGfx.set_sprite(asset_name, index, x, y, tile_id, palette_id, active, flip_x, flip_y, priority);
|
return LowGfx.set_sprite(bank_id, index, x, y, tile_id, palette_id, active, flip_x, flip_y, priority);
|
||||||
}
|
}
|
||||||
|
|
||||||
fn draw_text(x: int, y: int, message: str, color: Color) -> void
|
fn draw_text(x: int, y: int, message: str, color: Color) -> void
|
||||||
|
|||||||
@ -627,6 +627,57 @@ class PBSFrontendPhaseServiceTest {
|
|||||||
assertTrue(irBackend.getExecutableFunctions().stream().anyMatch(function -> "frame".equals(function.callableName())));
|
assertTrue(irBackend.getExecutableFunctions().stream().anyMatch(function -> "frame".equals(function.callableName())));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldLowerSdkGfxSetSpriteFacadeUsingBankIdContract() throws IOException {
|
||||||
|
final var projectRoot = tempDir.resolve("project-bootstrap-sdk-gfx-set-sprite");
|
||||||
|
final var sourceRoot = projectRoot.resolve("src");
|
||||||
|
final var modulePath = sourceRoot.resolve("app");
|
||||||
|
Files.createDirectories(modulePath);
|
||||||
|
|
||||||
|
final var sourceFile = modulePath.resolve("source.pbs");
|
||||||
|
final var modBarrel = modulePath.resolve("mod.barrel");
|
||||||
|
Files.writeString(sourceFile, """
|
||||||
|
import { Gfx } from @sdk:gfx;
|
||||||
|
|
||||||
|
fn frame() -> int
|
||||||
|
{
|
||||||
|
return Gfx.set_sprite(2, 0, 12, 18, 7, 3, true, false, true, 1);
|
||||||
|
}
|
||||||
|
""");
|
||||||
|
Files.writeString(modBarrel, "pub fn frame() -> int;");
|
||||||
|
|
||||||
|
final var projectTable = new ProjectTable();
|
||||||
|
final var fileTable = new FileTable(1);
|
||||||
|
final var projectId = projectTable.register(ProjectDescriptor.builder()
|
||||||
|
.rootPath(projectRoot)
|
||||||
|
.name("app")
|
||||||
|
.version("1.0.0")
|
||||||
|
.sourceRoots(ReadOnlyList.wrap(List.of(sourceRoot)))
|
||||||
|
.build());
|
||||||
|
|
||||||
|
registerFile(projectId, projectRoot, sourceFile, fileTable);
|
||||||
|
registerFile(projectId, projectRoot, modBarrel, fileTable);
|
||||||
|
|
||||||
|
final var ctx = new FrontendPhaseContext(
|
||||||
|
projectTable,
|
||||||
|
fileTable,
|
||||||
|
new BuildStack(ReadOnlyList.wrap(List.of(projectId))),
|
||||||
|
1);
|
||||||
|
final var diagnostics = DiagnosticSink.empty();
|
||||||
|
|
||||||
|
final var irBackend = new PBSFrontendPhaseService().compile(
|
||||||
|
ctx,
|
||||||
|
diagnostics,
|
||||||
|
LogAggregator.empty(),
|
||||||
|
BuildingIssueSink.empty());
|
||||||
|
|
||||||
|
assertTrue(diagnostics.stream().noneMatch(d ->
|
||||||
|
d.getCode().equals(PbsSemanticsErrors.E_SEM_EXEC_LOWERING_UNRESOLVED_CALLEE.name())));
|
||||||
|
assertTrue(irBackend.getExecutableFunctions().stream().anyMatch(function -> "frame".equals(function.callableName())));
|
||||||
|
assertTrue(irBackend.getReservedMetadata().hostMethodBindings().stream()
|
||||||
|
.anyMatch(h -> h.ownerName().equals("LowGfx") && h.sourceMethodName().equals("set_sprite")));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void shouldReportInconsistentBarrelInsideSdkBootstrapFixture() throws IOException {
|
void shouldReportInconsistentBarrelInsideSdkBootstrapFixture() throws IOException {
|
||||||
final var projectRoot = tempDir.resolve("project-stdlib-barrel-inconsistent");
|
final var projectRoot = tempDir.resolve("project-stdlib-barrel-inconsistent");
|
||||||
|
|||||||
@ -9,3 +9,11 @@ dependencies {
|
|||||||
implementation(project(":prometeu-compiler:prometeu-frontend-api"))
|
implementation(project(":prometeu-compiler:prometeu-frontend-api"))
|
||||||
implementation(project(":prometeu-compiler:prometeu-frontend-registry"))
|
implementation(project(":prometeu-compiler:prometeu-frontend-registry"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
tasks.register<JavaExec>("runCompile") {
|
||||||
|
group = "application"
|
||||||
|
description = "Runs p.studio.compiler.Compile against test-projects/main."
|
||||||
|
classpath = sourceSets.main.get().runtimeClasspath
|
||||||
|
mainClass = "p.studio.compiler.Compile"
|
||||||
|
workingDir = rootDir
|
||||||
|
}
|
||||||
|
|||||||
Binary file not shown.
@ -6,5 +6,5 @@
|
|||||||
"app_version": "0.1.0",
|
"app_version": "0.1.0",
|
||||||
"app_mode": "Game",
|
"app_mode": "Game",
|
||||||
"entrypoint": "0",
|
"entrypoint": "0",
|
||||||
"capabilities": ["log", "gfx"]
|
"capabilities": ["log", "gfx", "asset"]
|
||||||
}
|
}
|
||||||
|
|||||||
Binary file not shown.
@ -2,5 +2,6 @@
|
|||||||
|
|
||||||
set -e
|
set -e
|
||||||
|
|
||||||
|
cp build/assets.pa cartridge
|
||||||
cp build/program.pbx cartridge
|
cp build/program.pbx cartridge
|
||||||
./runtime/prometeu run cartridge
|
./runtime/prometeu run cartridge
|
||||||
@ -9,7 +9,8 @@ fn frame() -> void
|
|||||||
let touch = Input.touch();
|
let touch = Input.touch();
|
||||||
|
|
||||||
Gfx.clear(new Color(6577));
|
Gfx.clear(new Color(6577));
|
||||||
Gfx.draw_square(touch.x() - 8, touch.y() - 8, 16, 16, new Color(65535), new Color(13271));
|
// Gfx.draw_square(touch.x() - 8, touch.y() - 8, 16, 16, new Color(65535), new Color(13271));
|
||||||
|
let sprite_status = Gfx.set_sprite(0, 0, touch.x() - 8, touch.y() - 8, 0, 1, true, false, false, 0);
|
||||||
|
|
||||||
let a = 10;
|
let a = 10;
|
||||||
let b = 15;
|
let b = 15;
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user