Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RISC-V Target is missing support for -mabi=lp64d based systems #749

Closed
apcameron opened this issue Jun 19, 2023 · 8 comments · Fixed by #750
Closed

RISC-V Target is missing support for -mabi=lp64d based systems #749

apcameron opened this issue Jun 19, 2023 · 8 comments · Fixed by #750

Comments

@apcameron
Copy link

RISC-V Target is missing support for -mabi=lp64d based systems such as the Starfive Visionfive 2 Processor

-mabi=lp64d -march=rv64imafdc_zicsr_zba_zbb -mcpu=sifive-u74 -mtune=sifive-7-series.

The rv64i does not seem to build on it
It gives the following error
StarFive ~/Downloads/blis # make
Generating monolithic blis.h..........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
Generated include/rv64i/blis.h
Compiling obj/rv64i/config/rv64i/bli_cntx_init_rv64i.o ('rv64i' CFLAGS for config code)
In file included from /usr/include/features.h:515,
from /usr/include/bits/libc-header-start.h:33,
from /usr/include/stdio.h:27,
from ./frame/include//bli_system.h:46,
from config/rv64i/bli_cntx_init_rv64i.c:35:
/usr/include/gnu/stubs.h:14:11: fatal error: gnu/stubs-lp64.h: No such file or directory

The reason for this is it needs to include stubs-lp64d.h for this processor.

@angsch
Copy link
Collaborator

angsch commented Jun 20, 2023

The fastest workaround that I see is that you manually modify

CMISCFLAGS := -march=$(shell $(CC) -E frame/base/bli_riscv_detect_arch.h | grep '^[^\#]') -mabi=lp64

and revise the ABI to -mabi=lp64d.

You raise a good point here. There is not ABI detection. The RISC-V targets choose what we assumed is the most common one. For a generic rv64i target, we assumed this to be lp64.

@leekillough Do you have more thoughts on this?

@leekillough
Copy link
Collaborator

I will add ABI auto-detection, just like architecture auto-detection which already exists ( frame/base/bli_riscv_detect_arch.h ), but the stubs symptom is more of a toolchain issue. In my toolchain build script, I have:

build_riscv_toolchain() (
    cd -- "$RISCV/$RISCV_TOOLCHAIN"
    make -j16 linux

    if [[ $RISCV_COMPILER = llvm ]]; then
        if [[ $RISCV_SIZE = 64 ]]; then
            ln -sf stubs-lp64d.h "$RISCV/sysroot/usr/include/gnu/stubs-lp64.h"
            ln -sf stubs-lp64d.h "$RISCV/sysroot/usr/include/gnu/stubs-lp64f.h"
        else
            ln -sf stubs-ilp32d.h "$RISCV/sysroot/usr/include/gnu/stubs-ilp32.h"
            ln -sf stubs-ilp32d.h "$RISCV/sysroot/usr/include/gnu/stubs-ilp32f.h"
        fi
    fi
)

@apcameron
Copy link
Author

The workaround of changing line 49 to -mabi=lp64d does work for me and the checks all pass but it does not use the CPU's full feature set of rv64imafdc_zicsr_zba_zbb as I think your code its restricted to rv64i so it may run slower thank it could.

@leekillough
Copy link
Collaborator

The code currently detects rv64imafdc and is not restricted to rv64i, but I will add zba, zbb, zbc, zbs, and zfh, since those are detectable at compile-time. Zicsr and Zifencei are not currently detectable at compile-time with the latest C API, but Q->D->F implies Zicsr.

Clang does not enable the __riscv_v or __riscv_vector macros or choose -march=...v by default, even if it's built with vectors enabled, so we use a workaround to force vectors with -march=...v in the rv32iv and rv64iv configurations.

You can test the autodetection method this way:

$CC -E frame/base/bli_riscv_detect_arch.h | grep '^[^\#]'

-DFORCE_RISCV_VECTOR is specified for rv32iv and rv64iv because Clang does not enable vectors by default even if it's capable of them.

I will submit a PR shortly.

@apcameron
Copy link
Author

Thank you. Do you plan to create a release in the near future with the RISC V code included?

@leekillough
Copy link
Collaborator

Thank you. Do you plan to create a release in the near future with the RISC V code included?

I think BLIS is mostly released on a continuous integration model, so the latest master has the RISC-V code already, modulo the ABI header stub issues you've run into, and as soon as a PR is merged, it will have the changes.

@apcameron
Copy link
Author

That is correct but some distributions like GENTOO tend to wait for an official release before providing the updated package.

@leekillough
Copy link
Collaborator

@apcameron: In case it wasn't clear, rv32i, rv32iv, rv64i and rv64iv are base RISC-V configurations, since 32/64-bit and vector/non-vector, are the major categories which need to be separated. They are not the -march= flags passed to the compiler. Whether a particular RISC-V target supports M, A, F, D, C, etc., is not a part of the main BLIS configuration name, but gets automatically determined and used at build-time. It would be too hard to enumerate every possible combination of RISC-V extensions into the BLIS configuration name, so there are only 4 main ones.

The BLIS RISC-V implementation intentionally does not pass floating-point to assembly functions by value, so the "base" ABI of ilp32 or lp64 should be sufficient, which is why they were used in the makefiles. If you add the stubs to the toolchain with the symlink commands I listed above, then you should not get the missing stubs build error when BLIS uses -mabi=lp64.

PR750 should make the missing headers error go away, and may negligibly improve performance by using ilp32d or lp64d when possible.

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 a pull request may close this issue.

3 participants