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

wrong define of 128-bit related functions like __multi3 and __floatuntidf on csky #551

Closed
Dirreke opened this issue Oct 23, 2023 · 4 comments · Fixed by rust-lang/rust#117154

Comments

@Dirreke
Copy link

Dirreke commented Oct 23, 2023

I have added csky-unknown-linux-gnuabiv2 and csky-unknown-linux-gnuabiv2hf last days in rust-lang/rust#113658 . And I got SIGSEGV when I run a executable file on csky.

I checked the file and found that in LLVM-IR file of compiler-buildins, (Taking __floatuntidf as an example),it defines __floatuntidf as

define hidden noundef double @__floatuntidf(ptr noalias nocapture noundef readonly align 4 dereferenceable(16) %i) unnamed addr 

which has a ptr parameter but not values.

In executable files, the value of parameters are passed to the __floatuntidf. and then it regard the value as an address to load data, which causes the SIGSEGV


It's confused me that what caused the parameters become ptr.
Is this function compiled from float/conv.rs or anywhere else?
And how can I fixed it?

@bjorn3
Copy link
Member

bjorn3 commented Oct 23, 2023

What is the ABI for this function supposed to be? i128 can't be passed in a single register, so is it split into two registers or is it handled in a different way? And is this handling consistent with the way it is passed for regular functions or will it need a similar hack to #[unadjusted_on_win64] in the intrinsics! macro?

@Dirreke
Copy link
Author

Dirreke commented Oct 24, 2023

in the other platform like arm, it's defined as

define hidden double @_ZN17compiler_builtins5float4conv13__floatuntidf17h61afb9c475eb7bc7E(i128 %i) unnamed_addr #1

i128 will be divided to 4 i32 and passed to the __floatuntidf.

Passing the value to functions but not address is consitent with the funciton define in float/conv.rs

@bjorn3
Copy link
Member

bjorn3 commented Oct 24, 2023

Passing the value to functions but not address is consitent with the funciton define in float/conv.rs

The CSky C ABI definition rustc specifies that every type larger than 64bit and every aggregate type is to be passed as by-ref: https://github.com/rust-lang/rust/blob/271dcc1d40b57ad129d88fef1aa7f239308fbfb4/compiler/rustc_target/src/abi/call/csky.rs#L13-L14 Based on reading the ABI docs it is incorrect for both 128bit ints and aggregate types. These are to be passed in registers where possible and spilling onto the stack when there are not enough argument registers. And for returning it uses registers where possible and a return value pointer when larger than 64bit. You need to reproduce the entire ABI handling logic of clang at https://github.com/llvm/llvm-project/blob/4a074f32a6914f2a8d7215d78758c24942dddc3d/clang/lib/CodeGen/Targets/CSKY.cpp#L76-L162 in rustc.

@Dirreke
Copy link
Author

Dirreke commented Oct 24, 2023

Thanks! I will close it and make a PR in rust in a few days

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.

2 participants