IRC channel logs

2025-03-01.log

back to list of logs

<matrix_bridge><gtker> stikonas, oriansj: Opinions on abstracting away some of the compiler "backend" stuff so that if you need to push a register you would do something like "emit_push(REGISTER_ONE)" instead of a giant if block? IMO it's a little difficult to add new codegen because every instruction is at least 6 lines of if statements even if you only want to push/pop/load something
<matrix_bridge><gtker> As far as I can tell all the platforms could be abstracted relatively well away with something like 3 general purpose registers and some base pointer/stack pointer registers
<matrix_bridge><gtker> Codegen also seems a little inconsistent sometimes. There's 3 different ways of loading a constant and some of them treat different archs as if they need different instructions depending on the range of the value. "constant_load" just does "LOADI R0 x" for KNIGHT, but "primary_expr_number" does "LOADR R0 4\nJUMP4\n" if the value is outside of [32767;-32768]. The RISCs also do things a little differently
<mihi>gtker: About abstracting the arch-specific codegen parts into separate methods: I've been thinking about that as well. I don't think it matters if you have codegen_* or emit_* methods. Ultimately did not do it due to lack of time, and I think the arithmetic_recursyion logic would become more complicated.
<mihi>The "obvious" other choice would be to make M2-Planet emit (as the name implies in my opinion) platform neutral M1 code, but that would probably either require two M1 passes or a more complex M1 implementation.
<mihi>about varargs and calling convention: As the caller sets up the frame pointer, I think one could even live with the args on the stack. va_size can be determined from difference of stack pointer and frame pointer, and the first args are indexed by frame pointer anyway (today you already can call a function with more arguments than it declares and nothing breaks)
<matrix_bridge><gtker> I considered that as well, but thought that in the end keeping it inside M2-Planet would offer most of the benefits of a platform neutral assembly while still being able to output platform independt instructions if necessary
<mihi>(speaking of x86_32 architecture, but I assume the others are similar)
<matrix_bridge><gtker> mihi: I think local variables break if a function is called with more arguments than it takes
<matrix_bridge><gtker> (for x64)
<mihi>ok, possible. Maybe I misremember :D
<matrix_bridge><Andrius Štikonas> gtker: yeah, it would be useful to factor out some common codegen function
<matrix_bridge><Andrius Štikonas> not sure how exactly we should do it though
<matrix_bridge><Andrius Štikonas> whether it should be separate per arch file...
<matrix_bridge><Andrius Štikonas> or just some functions in cc_core.c...
<matrix_bridge><Andrius Štikonas> and yes, as we get more features, we do need this
<matrix_bridge><Andrius Štikonas> since we get more and more places with codegen output
<matrix_bridge><gtker> I've experimented a little and I think it would work as just separate functions abstract away the platform
<matrix_bridge><gtker> Some of the RISCV code does things slightly different than the others, but it should be able to make the abstraction relatively clean
<matrix_bridge><Andrius Štikonas> gtker: how about https://github.com/oriansj/M2libc/pull/71 for other arches?
<matrix_bridge><Andrius Štikonas> e.g. x86?
<matrix_bridge><Andrius Štikonas> or is it already there?
<matrix_bridge><Andrius Štikonas> well, RISC-V does not need any new defines...
<matrix_bridge><gtker> They're already there, AMD64 is the only one that doesn't have it. Instead it pushes and pops
<matrix_bridge><Andrius Štikonas> weird that we didn't have it, let me merge it
<matrix_bridge><Andrius Štikonas> if we are lucky, maybe abstarcting more things will fix unxz bug...
<matrix_bridge><gtker> That's what I'm thinking as well. There are 2-3 places where we load constants and some of them do checking for range and some do not
<matrix_bridge><gtker> stikonas: What are you using to test the RISCV stuff?
<matrix_bridge><Andrius Štikonas> mostly qemu
<matrix_bridge><Andrius Štikonas> occasionally I run it on visionfive2 board
<matrix_bridge><Andrius Štikonas> so qemu has userspace emulation mode
<matrix_bridge><gtker> Any guides to getting set up for testing M2-Planet specifically? Last I checked qemu seemed to want me to compile a compatible kernel and all sorts of stuff
<matrix_bridge><Andrius Štikonas> no, you don't need that
<matrix_bridge><Andrius Štikonas> that's qemu-system
<matrix_bridge><Andrius Štikonas> there is qemu-riscv64 binary that translates riscv64 syscalls with your native syscalls
<matrix_bridge><Andrius Štikonas> and can run directly on your system
<matrix_bridge><Andrius Štikonas> as for binfmt.d
<matrix_bridge><Andrius Štikonas> you can add a hook to /etc/binfmt.d/qemu-riscv64-static.conf
<matrix_bridge><Andrius Štikonas> ":riscv64:M::\x7fELF\x02\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\xf3\x00:\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff:/usr/bin/qemu-riscv64:"
<matrix_bridge><Andrius Štikonas> and that will just let you run riscv binaries
<matrix_bridge><gtker> Hmm, alright, I'll check it out if I find the time 🙂
<matrix_bridge><Andrius Štikonas> some distros might have some packages to help with that
<matrix_bridge><Andrius Štikonas> though I think I created that file myself on Gentoo
<matrix_bridge><Andrius Štikonas> but e.g. Arch has https://archlinux.org/packages/extra/x86_64/qemu-user-static-binfmt/
<matrix_bridge><gtker> I'm on Ubuntu 22, don't think there's an equivalent 😕
<matrix_bridge><Andrius Štikonas> there is binfmt-support package but I don't yet know what it includes
<matrix_bridge><Andrius Štikonas> you'll probably have to create conf files yourself then
<matrix_bridge><Andrius Štikonas> there is a guide for Debian https://wiki.debian.org/QemuUserEmulation
<matrix_bridge><Andrius Štikonas> should be mostly the same
<matrix_bridge><gtker> Is there a way to make the test script also run non-platform binaries?
<matrix_bridge><Andrius Štikonas> yes, there is some env variable
<matrix_bridge><Andrius Štikonas> for M2-Planet make test?
<matrix_bridge><gtker> Yeah
<matrix_bridge><Andrius Štikonas> I guess ARCH
<matrix_bridge><Andrius Štikonas> and that comes from if [ "$(get_machine ${GET_MACHINE_FLAGS})" = "${ARCH}" ]
<matrix_bridge><Andrius Štikonas> (that's in each run_test.sh)
<matrix_bridge><Andrius Štikonas> so you need to add get_machine to your path
<matrix_bridge><Andrius Štikonas> I think that's mescc-tools-extra
<matrix_bridge><Andrius Štikonas> no, maybe just mescc-tools
<matrix_bridge><gtker> It's the same one that has "blood-elf" and "M1"? I've already got that one
<matrix_bridge><Andrius Štikonas> "./get_machine --override riscv64" prints riscv64
<matrix_bridge><Andrius Štikonas> yeah
<matrix_bridge><Andrius Štikonas> so "GET_MACHINE_FLAGS="--override riscv64" should work I think
<matrix_bridge><Andrius Štikonas> * riscv64""
<matrix_bridge><Andrius Štikonas> gtker: hmm, there is a regression in risc-v
<matrix_bridge><Andrius Štikonas> both on 32 and 64-bits
<matrix_bridge><gtker> What's the problem?
<matrix_bridge><Andrius Štikonas> +> ./riscv32/artifact/blood-elf-0 --little-endian -f ./riscv32/artifact/M1-macro-0.M1 -o ./riscv32/artifact/M1-macro-0-footer.M1 crashes
<matrix_bridge><Andrius Štikonas> let me test before your PR...
<stikonas>and blood-elf I think is the first binary that is built by M2-Planet
<stikonas>reverting before that last PR fixes it
<matrix_bridge><gtker> https://github.com/gtker/M2-Planet/blob/36f9db958c2c7cec3fbdd2b95b7f28c5f7841aaf/cc_core.c#L472 It might be here. If you remove the TEMP after BASE does it work?
<stikonas>let's see
<stikonas>no, doesn't help...
<stikonas>I could bisect to see which commits breaks it
<stikonas>it's a bit slow on qemu, but we don't have many commits
<matrix_bridge><gtker> Can you get a gdb stacktrace?
<stikonas>yeah, I can...
<matrix_bridge><gtker> I'm pretty sure the TEMP that you removed needs to be removed no matter what. Since the original assembly is
<matrix_bridge><gtker> emit_out("rd_sp rs1_sp !-24 addi\t# Allocate stack\n");
<matrix_bridge> emit_out("rs1_sp rs2_ra @8 sd\t# Protect the old return pointer\n");
<matrix_bridge> emit_out("rs1_sp rs2_fp sd\t# Protect the old frame pointer\n");
<matrix_bridge> emit_out("rs1_sp rs2_tp @16 sd\t# Protect temp register we are going to use\n");
<matrix_bridge> emit_out("rd_tp rs1_sp mv\t# The base pointer to-be\n");
<stikonas>oh it's useless...
<stikonas>that's before any debug info at all
<matrix_bridge><gtker> That reads to me like push frame pointer first, then return pointer, and then temp pointer?
<stikonas>we don't have blood elf
<matrix_bridge><gtker> Ah, damn
<stikonas>well, I could build it with gcc
<matrix_bridge><gtker> Or is that the other way around? Temp, return, then base?
<stikonas>well, maybe not with gcc, but manually add another blood-elf call
<stikonas>hmm, let me see
<stikonas>it was return base temp before
<matrix_bridge><gtker> Was it? The base is index 0, return is 8 and temp is 16?
<matrix_bridge><gtker> They're just in a slightly weird order?
<stikonas>oh maybe you are right
<stikonas>they are out of order
<stikonas>so it's then base return temp
<matrix_bridge><gtker> Ah, should the base then be shifted one line down instead of being deleted?
<stikonas>I think so, let me check pop's at the end too
<stikonas>and then I'll test it
<stikonas>and it's definitely that commit ba8dd21d4df25f180942c1e4f667fc0025b3a2de
<stikonas>no, just moving TEMP down didn't help
<stikonas>back to looking at the code...
<matrix_bridge><gtker> Hmm. I've gotta go sleep now but if you post a stacktrace I'll see if I can figure it out tomorrow
<stikonas>sure
<stikonas>it might be reverse order
<stikonas>I'll figure it out
<stikonas>and push it...
<stikonas>it can't be that hard, we know exactly which commit broke it
<stikonas>(just a bit slow to test since it's qemu)
<matrix_bridge><Andrius Štikonas> hmm, though the new code is less efficient ☹️
<matrix_bridge><Andrius Štikonas> before we were doing addi -24, now tree times addi -8. Same for pops and that's for each function call...