Stream: compiler development

Topic: stale nightlies

Anton (Dec 11 2023 at 15:03):

No new nightlies have been uploaded in the last two days because of hanging gen tests on macos apple silicon. They don't hang when using nix but they do without. gen_abilities::inspect::num and gen_abilities::inspect::list always hang. Going to bisect now.

Anton (Dec 11 2023 at 15:13):

This is coming from PR#6216, I'll see if can narrow it down further.

Brendan Hansknecht (Dec 11 2023 at 15:35):

Almost certainly the compiler rt changes somehow

Anton (Dec 11 2023 at 15:35):

The breaking change is either coming from e621de3, for which the test fails with:

thread 'gen_abilities::inspect::num' panicked at 'Unrecognized builtin function: "longjmp" - if you're working on the Roc compiler, do you need to rebuild the bitcode? See compiler/builtins/bitcode/README.md', crates/compiler/gen_llvm/src/llvm/bitcode.rs:63:28

Or from 1ad9933 where it's stuck for the first time.

Anton (Dec 11 2023 at 15:37):

The test also runs a lot slower starting with be06599, it went from ~0.7s to ~3.4s for the single num test.

Brendan Hansknecht (Dec 11 2023 at 15:39):

So I first changed things in a way that accidentally made things way slower. Then later commits fixed that

Brendan Hansknecht (Dec 11 2023 at 15:42):

So the e632de3 commit should have fixed test execution time (maybe still a minor regression but should be very small)

Brendan Hansknecht (Dec 11 2023 at 15:43):

1ad9933 theoretically re-added the symbol needed for long jump, but maybe it didn't get added fully correctly.

Brendan Hansknecht (Dec 11 2023 at 15:44):

Also weird it just affects those two tests

Brendan Hansknecht (Dec 11 2023 at 15:45):

Is the zig version correct outside of nix? Could there be some sort of versioning issue?

Anton (Dec 11 2023 at 16:05):

zig is on 0.11.0

Brendan Hansknecht (Dec 11 2023 at 16:06):

Can you share the builtins-host.ll

Brendan Hansknecht (Dec 11 2023 at 16:07):

Should be in compiler/builtins/bitcode/zig-out

Brendan Hansknecht (Dec 11 2023 at 16:07):

Oh, I guess I should double check, this are failing with the llvm backend specifically?

Anton (Dec 11 2023 at 16:08):

builtins-host.ll

Anton (Dec 11 2023 at 16:08):

Oh, I guess I should double check, this are failing with the llvm backend specifically?

I'm running cargo test --release -p test_gen gen_abilities::inspect::num I think that uses llvm

Anton (Dec 11 2023 at 16:09):

Brendan Hansknecht said:

Also weird it just affects those two tests

It also happens with gen_list and gen_compare. The gen_abilities failures are the first I could reliably reproduce.

Brendan Hansknecht (Dec 11 2023 at 16:21):

hmm... nothing looks to have change around setjmp or longjmp in the bitcode.

Brendan Hansknecht (Dec 11 2023 at 16:25):

can you try running on testing-for-nightlies

Anton (Dec 11 2023 at 16:30):

Success :)

Brendan Hansknecht (Dec 11 2023 at 17:03):

Interesting....

Brendan Hansknecht (Dec 11 2023 at 17:03):

Can you also do a quick perf check vs main. This is theoretically slightly slower than my other fix

Brendan Hansknecht (Dec 11 2023 at 17:04):

Also, I wonder what we are DCEing wrong

Anton (Dec 11 2023 at 17:25):

for gen_abilities_inspect::num it's ~0.42s on main and ~0.6s on testing-for-nightlies. Will do another one with all of gen_abilities.

Anton (Dec 11 2023 at 17:29):

For all of gen_abilities there is no difference

Brendan Hansknecht (Dec 11 2023 at 17:32):

When you get the chance can you pull again and just test the functionality of the branch?

Anton (Dec 11 2023 at 18:12):

It's stuck again unfortunately

Anton (Dec 11 2023 at 18:44):

I'm done for today, but it should be reproducible locally (outside nix) on any apple silicon device. Setting up the dependencies without nix is pretty easy:

brew install llvm@16
brew install zig
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
export LLVM_SYS_160_PREFIX=$(brew --prefix llvm@16)

Brendan Hansknecht (Dec 11 2023 at 18:46):

I actually just cleaned up the commit and am just gonna PR the removal of early DCE. Should fix the issue.

Brendan Hansknecht (Dec 11 2023 at 18:46):

https://github.com/roc-lang/roc/pull/6254

Brendan Hansknecht (Dec 11 2023 at 18:47):

Unless I mixed something up, this should be equivalent to what passed before, but cleaned up into a PR.

Anton (Dec 12 2023 at 14:57):

cargo test --release -p test_gen gen_abilities::inspect::num is still stuck on latest main. Will check out the diff with testing-for-nightlies.

Anton (Dec 12 2023 at 15:13):

The testing-for-nightlies branch only had the following removed:

    let mpm = PassManager::create(());
    mpm.add_global_dce_pass();
    mpm.run_on(&module);

PR#6254 does have some extra changes

Brendan Hansknecht (Dec 12 2023 at 15:34):

Interesting. I guess some more functions need to be external even if dce is late

Brendan Hansknecht (Dec 12 2023 at 18:46):

#6263 works locally for me outside of nix.

Anton (Dec 13 2023 at 10:34):

All seemed well but the basic-cli tests revealed an error:

thread 'main' panicked at 'Undefined Symbol in relocation, (+925, Relocation { kind: PltRelative, encoding: Generic, size: +20, target: Symbol(SymbolIndex(+5e)), addend: +fffffffffffffffc, implicit_addend: false }): Ok(Symbol { name: "__udivti3", address: +0, size: +0, kind: Label, section: Undefined, scope: Unknown, weak: false, flags: Elf { st_info: +10, st_other: +0 } })', crates/linker/src/elf.rs:1486:25

__udivti3 appears to be another special function

Anton (Dec 13 2023 at 10:57):

I'll add it to must_keep and see if that fixes things

Anton (Dec 13 2023 at 11:08):

It does :)

Richard Feldman (Dec 13 2023 at 12:33):

that function name sounds familiar...I'm pretty sure we've run into issues involving it before

Anton (Dec 13 2023 at 12:49):

Yes indeed, it's mentioned in #3609 and John was seemingly the first to find the current bug 3 days ago in #6245

Brendan Hansknecht (Dec 13 2023 at 16:02):

Weird. That definitely should not be a must keep function. It should only be needed if a used built-in calls it.

Brendan Hansknecht (Dec 13 2023 at 16:46):

Maybe it is required cause somehow __udivti3 is generated by an llvm instrisic call. That is general what creates must keep functions. They don't think they are used until the llvm instrisics resolve to real function calls. So DCE removes them.

Anton (Dec 13 2023 at 17:23):

Can someone approve #6270 so I can release nightlies again?

Last updated: Jul 06 2025 at 12:14 UTC