Add support for LoongArch32#957
Conversation
atgreen
left a comment
There was a problem hiding this comment.
Thanks for the LoongArch32 work! A few items worth discussing:
-
Memory layout change in
ffi_call_intaffects LP64 too — The introduction ofextra_bytesand the change tocb.stackinitialization (cb.stack = (void *) alloc_base + extra_bytes + rval_bytes) diverges from the current layout wherecb.stackandcb.used_stackboth start atalloc_base. This changes behavior for 64-bit as well, not just 32-bit. Could you explain why this layout change is needed for LA32, and confirm it doesn't regress LP64? -
Trampoline size assumptions — The trampoline code table uses
.rept 65536 / 16— do the size assumptions hold on 32-bit where pointer sizes differ? -
LOONGARCH32target define appears unused —configure.hostsetsTARGET=LOONGARCH32vsTARGET=LOONGARCH64, butffi.cdoesn't branch on this anywhere. The ABI selection relies entirely on__loongarch_grlenfrom the compiler, which is fine, but is theTARGETdistinction used elsewhere (e.g. Makefile conditionals), or is it dead?
Thanks for your review. Some replies below.
|
The size of the parameter block must be smaller than the size of the trampoline, so the generic code can guarantee the offset of the parameter block is the same as the offset of the trampoline. Given the parameter block of LA32 must be smaller than the one of LA64 things should be fine. |
|
Thanks for the replies and test results! Two items I'd like addressed before merging:
|
Change src/loongarch64 to src/loongarch. Change __loongarch64 to __loongarch_grlen. __loongarch64 is uesd for compatibility with legacy code, new programs should not assume existence of this macro.[1] And there is no __loongarch32 macro for LoongArch32. Add ilp32s, ilp32f, ilp32d abi. Add ADDI macro. [1] https://github.com/loongson/la-toolchain-conventions?tab=readme-ov-file#cc-preprocessor-built-in-macro-definitions
CHECK(a8 == 8) fail on LoongArch32. long double passed by reference on LoongArch32 ilp32 ABI. If a argument register is available, the address is passed in the argument register; otherwise, it is passed on the stack[1]. The address of return value and a1-a7 arguments of cls_ldouble_fn passed by a0-a7 argument registers. The address of a8 argument need to be passed by stack. But ffi_call_int only allocates cif->bytes(sizeof(long double)*8) bytes space for stack. Both the address of a8 argument and a8 argument are saved at sp+0 address. The lowest 4-byte of a8 argument are overwritten by the address of a8 argument. Allocates extra conservative estimate space like RISC-V. [1] https://github.com/loongson/la-abi-specs/blob/release/lapcs.adoc#passing-arguments
Thanks for the feedback! I’ve addressed both items and rebased to master branch. |
Change src/loongarch64 to src/loongarch.
Change __loongarch64 to __loongarch_grlen.
__loongarch64 is used for compatibility with legacy code, new programs should not assume existence of this macro.[1] And there is no __loongarch32 macro for LoongArch32.
Add ilp32s, ilp32f, ilp32d abi.
Add ADDI macro.
[1] https://github.com/loongson/la-toolchain-conventions?tab=readme-ov-file#cc-preprocessor-built-in-macro-definitions