Skip to content

nock: simple branch elimination#850

Merged
pkova merged 6 commits intonext/kelvin/409from
dozreg/eliminate-branches
Jul 23, 2025
Merged

nock: simple branch elimination#850
pkova merged 6 commits intonext/kelvin/409from
dozreg/eliminate-branches

Conversation

@dozreg-toplud
Copy link
Copy Markdown
Contributor

Adds simple branch elimination during nock compilation.

Together with urbit/urbit#7203, ~? is compiled to just three ops when verb is set to |, and there are no branching instructions if it is set to either & or |:

{lit0 snol tall [lilb 42] halt}

> =*  a  |
  ~>  %xray
  ~?  a  42  42
42

@dozreg-toplud dozreg-toplud requested a review from a team as a code owner July 18, 2025 18:43
@dozreg-toplud dozreg-toplud force-pushed the dozreg/eliminate-branches branch from 348fb46 to 637c270 Compare July 19, 2025 08:30
@dozreg-toplud dozreg-toplud requested a review from joemfb July 21, 2025 15:18
Comment thread pkg/noun/retrieve.c Outdated
@dozreg-toplud dozreg-toplud requested a review from joemfb July 21, 2025 15:26
@pkova pkova merged commit 4da7386 into next/kelvin/409 Jul 23, 2025
2 checks passed
@pkova pkova deleted the dozreg/eliminate-branches branch July 23, 2025 15:31
pkova added a commit that referenced this pull request Sep 19, 2025
I realized that branch elimination introduced in #850 led to crashes
like in this example:

```
.*(0 [%6 [%5 [%0 1] %1 0] [%1 42] [%6 [%1 %0] [%0 0 0] [%1 42]]])
```

When outer %6 is compiled, `_n_formulaic` checks the validity of
formulas in branches, replacing them with single `BAIL` instruction if
the formula is malformed, which would make it always crash. From POV of
_n_formulaic inner %6 is valid, since the condition expression and at
least one of the branch expressions are valid. But when inner %6 is
compiled, we only compile the invalid branch due to branch elimination,
leading to a crash.

For that reason _n_formulaic has to be more strict in the presence of
branch elimination.
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.

3 participants