Ahmad Sattar (Jan 17 2023 at 17:46): Ahmad Sattar (Jan 17 2023 at 17:46):Hey, I recently started looking into roc-lang and want to contribute to the project!
I have previously worked a little on other compilers in Rust, and would be interested in helping wherever I can! :)
I've been eyeing helping on the gen-dev backend through some of the "good first issue"s for that, for example https://github.com/roc-lang/roc/issues/3796
I want to sanity checking that I wouldn't be stepping on any toes of anyone already looking to work on the issue, and I would also like to ask for a little bit of general guidance <3
For now I have tried to just copy the existing NumSub generation at https://github.com/roc-lang/roc/compare/main...thehabbos007:roc:asa/numsubwrap and gotten test-gen-dev to run
Folkert de Vries (Jan 17 2023 at 17:51):help there is very welcome!
Folkert de Vries (Jan 17 2023 at 17:52):the usual procedure is to write some c/rust/zig in https://godbolt.org/ to show us the assembly that we want to get
Folkert de Vries (Jan 17 2023 at 17:53):and then make that happen. For numerics in particular it's good to check it for a few different types (widths) because the assembly can change sometimes
Ahmad Sattar (Jan 17 2023 at 18:02):Cool, thanks! Will make sure to use that for opcodes. Are there any good reference sites other than the compiler explorer?
Ahmad Sattar (Jan 17 2023 at 18:04):Also it seems that optimizations usually optimize away the interesting bits (for rust at least). Other than something like -O0 can I do anything?
Folkert de Vries (Jan 17 2023 at 18:06):yeah, I think https://doc.rust-lang.org/stable/std/hint/fn.black_box.html is what you're looking for
Folkert de Vries (Jan 17 2023 at 18:06):or just don't provide input values at all
Folkert de Vries (Jan 17 2023 at 18:08):e.g.
// Type your code here, or load an example.
pub fn square(a: i32, b: i32) -> i32 {
a - b
}
will show you useful assembly with -O for rust
Folkert de Vries (Jan 17 2023 at 18:08):because there are no inputs, it cannot make weird assumptions based on the inputs
Brendan Hansknecht (Jan 17 2023 at 18:17):If you haven't, make sure to read through: https://github.com/roc-lang/roc/blob/main/crates/compiler/gen_dev/README.md
It has a lot of info and some links to useful tools.
Brendan Hansknecht (Jan 17 2023 at 18:18):Also, if you need help, feel free to reach out here or request to pair. I definitely could help run you through an addition.
Folkert de Vries (Jan 17 2023 at 18:19):for sub specifically, there is https://github.com/roc-lang/roc/pull/4101 but that one is pretty stale
Folkert de Vries (Jan 17 2023 at 18:21):that PR does not actually do anything, and there was some confusion about what test command to run. No real activity since september
Brendan Hansknecht (Jan 17 2023 at 18:21):Also, here is another useful doc: https://github.com/rtfeldman/roc/issues/1932#issuecomment-964860520
Ahmad Sattar (Jan 17 2023 at 18:27):Thanks Brendan for the links!
I suppose the PR linked is for the non-wrapping sub, which I assume is different?
Folkert de Vries (Jan 17 2023 at 18:31):technically, yes, but I believe that we don't do the overflow check yet for the dev backend
Folkert de Vries (Jan 17 2023 at 18:31):in roc, a + b should panic if that operation overflows, but the dev backend just used the add instruction, which will silently overflow
Ahmad Sattar (Jan 17 2023 at 18:32):I see. Should I look into another issue instead, then?
Folkert de Vries (Jan 17 2023 at 18:32):no, this one is fine, and subtraction is useful
Folkert de Vries (Jan 17 2023 at 18:33):there is just a lot to implement and we do it in small steps. The panicking is not really required to run most programs
Folkert de Vries (Jan 17 2023 at 18:34):and it is more complicated than just emitting some instructions; you have to call a panic function there. So we've skipped that so far
Ahmad Sattar (Jan 17 2023 at 18:36):I see. Would that to-be-added panic code be emitted as part of the build_* methods rather than the the specialized ASM::sub_reg... functions? I ask because I assume I wouldn't need to make a ASM::wrapping_sub_reg.. equivalent if we do the panics in the former step
Folkert de Vries (Jan 17 2023 at 18:38):yes the panic would wrap the lower level stuff
Folkert de Vries (Jan 17 2023 at 18:38):it's sort of if bunch_of_check { panic!() } else { do_the_normal_assembly }
Folkert de Vries (Jan 17 2023 at 18:38):in the general case
Folkert de Vries (Jan 17 2023 at 18:39):for addition/subtraction there are probably some flags that we could use to figure out if the addition overflowed
Ahmad Sattar (Jan 17 2023 at 18:40):Thanks for the explanation, that does make sense :)
Ahmad Sattar (Jan 17 2023 at 19:22):I opened a PR here https://github.com/roc-lang/roc/pull/4917, but no rush reviewing it of course!
Ahmad Sattar (Jan 21 2023 at 11:35):Hey, sort of unrelated to the above stuff, but still relevant to the topic: I've tried implementing the List.withCapcity builtin in the gen_dev backend, I changed an existing test to account for List capacity but it seems that the wasm backend doesn't have the expected capacity Does anyone have some insights as to why the wasm RocList seems to be different?
CI result: https://github.com/roc-lang/roc/actions/runs/3970756961/jobs/6809484103#step:12:1132
Folkert de Vries (Jan 21 2023 at 12:34):I suspect this is a problem with getting the value back from wasm to rust. based on the chrome debugger, within wasm, the capacity is 10
Folkert de Vries (Jan 21 2023 at 12:34):I'll see if I can fix this on your branch
Ahmad Sattar (Jan 21 2023 at 12:37):Thank you, I appreciate it :)
Folkert de Vries (Jan 21 2023 at 13:18):it's not that simple. Somehow something is wrong allocating. The wasm program takes a different route through the zig code than the llvm program, and in the process the capacity gets messed up
Ahmad Sattar (Jan 21 2023 at 13:22):Interesting. Should I just undo the test changes and put aside the wasm issue?
Folkert de Vries (Jan 21 2023 at 13:27):no I'm on it. There turns out to be a second issue, which is more in the area where I originally thought it would be
Ahmad Sattar (Jan 21 2023 at 13:27):Got it. Again, thank you!
Folkert de Vries (Jan 21 2023 at 14:59):allright, I found it. We were using a pointer as if it were an integer
Folkert de Vries (Jan 21 2023 at 15:00):that meant a list with a very large capacity was created
Ahmad Sattar (Jan 21 2023 at 15:01):Neat, nice find! Excited to see the piece of code this was in
Folkert de Vries (Jan 21 2023 at 15:09):can you rebase your branch on https://github.com/roc-lang/roc/tree/list-with-capacity? I suspect the "draft PR" does not work well with the gh tool, so it's hard for me to push to your branch
Ahmad Sattar (Jan 21 2023 at 15:11):Yes, can do. I can open up for review too
Folkert de Vries (Jan 21 2023 at 15:12):I did that already but it still did not work. I'm upgrading gh now, maybe that is what it is
Ahmad Sattar (Jan 21 2023 at 15:13):My commit is already in the branch, perhaps that's why it's a noop
Folkert de Vries (Jan 21 2023 at 15:13):but my commits aren't right?
Ahmad Sattar (Jan 21 2023 at 15:14):I mean that the https://github.com/roc-lang/roc/tree/list-with-capacity branch has my commit, so a rebase is a noop
Folkert de Vries (Jan 21 2023 at 15:15):right, ok then maybe just reset --hard my_branch on your branch, so the PR updates?
Ahmad Sattar (Jan 21 2023 at 15:17):There we go, thanks!
Last updated: Jul 06 2025 at 12:14 UTC