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

__aeabi_ functions should use the AAPCS calling convention, at least with LLVM 4.0+ #116

Closed
mattico opened this issue Nov 12, 2016 · 15 comments · Fixed by #141
Closed

__aeabi_ functions should use the AAPCS calling convention, at least with LLVM 4.0+ #116

mattico opened this issue Nov 12, 2016 · 15 comments · Fixed by #141

Comments

@mattico
Copy link
Contributor

mattico commented Nov 12, 2016

I'm not exactly sure about the specifics of this issue but based on
rust-lang/compiler-rt#26 (comment)
I think the __aeabi aliases should use the AAPCS calling convention. extern "aapcs" fn seems to work. If the functions that are being aliased don't also use extern "aapcs" there may be some shuffling around of registers which may hamper inlining.

@mattico
Copy link
Contributor Author

mattico commented Nov 12, 2016

Is there a better way to solve this than using a macro for every extern fn declaration?

@japaric
Copy link
Member

japaric commented Nov 12, 2016

I think the __aeabi aliases should use the AAPCS calling convention

Yes or we'll hit the floatundidf bug when we integrate this into rust-lang/rust.

If the functions that are being aliased don't also use extern "aapcs"

The "aliased" functions must also use extern "aapcs"

This may me realize that we may be invoking the functions in libcompiler-rt.a with the wrong calling convention. We are calling them as if they were extern fns when we should be using extern "aapcs" fn instead. I expect this would cause problems for the arm-gnueabihf target. Have we had problems with that target?

@japaric
Copy link
Member

japaric commented Nov 12, 2016

This may me realize that we may be invoking the functions in libcompiler-rt.a with the wrong calling convention

To clarify: I meant in our unit tests. We have a transmute from usize to extern fn.

@mattico
Copy link
Contributor Author

mattico commented Nov 12, 2016

Have we had problems with that target?

Yes, this seems very likely to be causing #90 and the issues in #106.

To clarify: I meant in our unit tests. We have a transmute from usize to extern fn.

If I recall correctly, the issues only occurred inside the unit tests not when called normally, which makes sense.

I'll go test this theory right now.

@mattico
Copy link
Contributor Author

mattico commented Nov 12, 2016

I tested this by making everything extern "aapcs" and making sure the compiler-rt submodule included the aapcs changes and it didn't fix anything. I'm probably just missing something since this still seems like the most likely culprit.

https://github.com/mattico/compiler-builtins/tree/aapcs

@japaric
Copy link
Member

japaric commented Nov 12, 2016

I don't think changes like these are correct:

fn __addsf3(f: extern "aapcs" fn(f32, f32) -> f32,

I think the intrinsics in libgcc.a are compiled the "hard" calling convention (not aapcs) so those should be called using extern "C" fn. I also think that extern "aapcs" fn doesn't make a difference over extern "C" fn in functions that don't take or return floating point values.

@mattico
Copy link
Contributor Author

mattico commented Nov 12, 2016

From the discussion above, I gather:

Lib eabi rest
gcc_s aapcs c
compiler-rt aapcs c
compiler-rt 4.0 aapcs aapcs
compiler-builtins c c

@TimNN
Copy link
Contributor

TimNN commented Nov 16, 2016

Note that for rust currently aapcs == c. Also, in llvm, there are actually aapcs and aapcs-vfp, where the vfp version is hard float and the "normal" version is soft float. Then, for llvm, c is aapcs on soft float targets and aapcs-vfp on hard float targets.

Af far as I can tell, in Rust there is currently no way to specify aapcs (not vfp) on armhf targets.

The only thing that changes for llvm 4.0, I think, is that the powi intrinsic now requires aapcs (no hf) now as well (it previously did not). This as been reported as a regression to llvm (https://llvm.org/bugs/show_bug.cgi?id=30543).

@parched
Copy link
Contributor

parched commented Nov 16, 2016

Af far as I can tell, in Rust there is currently no way to specify aapcs (not vfp) on armhf targets.

Does extern "aapcs" fn not work?

This as been reported as a regression to llvm

It's not clear to me whether that is a regression or improvement. I think it's regression but the compiler-rt guy thinks that's how it should be.

@TimNN
Copy link
Contributor

TimNN commented Nov 16, 2016

Af far as I can tell, in Rust there is currently no way to specify aapcs (not vfp) on armhf targets.

Does extern "aapcs" fn not work?

No, from the rust source:

Aapcs => llvm::CCallConv,

@japaric
Copy link
Member

japaric commented Nov 16, 2016

Aapcs => llvm::CCallConv,

Is that a bug? With that extern "aapcs" fn would be no different than extern "C" fn then ...

@TimNN
Copy link
Contributor

TimNN commented Nov 16, 2016

Is that a bug? With that extern "aapcs" fn would be no different than extern "C" fn then ...

Well, there's the comment // These API constants ought to be more specific... just above...

I would assume that this was the easy way of getting the correct aapcs version (soft vs hard float) without having to add that logic to rustc.

@parched
Copy link
Contributor

parched commented Nov 16, 2016

Yes definitely a bug and the reason the @mattico's test failed.

@parched
Copy link
Contributor

parched commented Nov 16, 2016

Bug report here rust-lang/rust#37810

@japaric
Copy link
Member

japaric commented Nov 27, 2016

@mattico FYI: extern "aapcs" fn should be fixed in recent nightlies, if you want to give this another try.

japaric pushed a commit that referenced this issue Feb 7, 2017
also, on ARM, inline(always) the actual implementation of the intrinsics so we
end with code like this:

```
00000000 <__aeabi_dadd>:
    (implementation here)
```

instead of "trampolines" like this:

```
00000000 <__aeabi_dadd>:
    (shuffle registers)
    (call __adddf3)

00000000 <__adddf3>:
    (implementation here)
```

closes #116
bors added a commit that referenced this issue Feb 7, 2017
use AAPCS calling convention on all aeabi intrinsics

also, on ARM, inline(always) the actual implementation of the intrinsics so we
end with code like this:

```
00000000 <__aeabi_dadd>:
    (implementation here)
```

instead of "trampolines" like this:

```
00000000 <__aeabi_dadd>:
    (shuffle registers)
    (call __adddf3)

00000000 <__adddf3>:
    (implementation here)
```

closes #116

cc #66
r? @alexcrichton
cc @mattico
bors added a commit that referenced this issue Feb 8, 2017
use AAPCS calling convention on all aeabi intrinsics

also, on ARM, inline(always) the actual implementation of the intrinsics so we
end with code like this:

```
00000000 <__aeabi_dadd>:
    (implementation here)
```

instead of "trampolines" like this:

```
00000000 <__aeabi_dadd>:
    (shuffle registers)
    (call __adddf3)

00000000 <__adddf3>:
    (implementation here)
```

closes #116

cc #66
r? @alexcrichton
cc @mattico
@bors bors closed this as completed in #141 Feb 8, 2017
frmdstryr referenced this issue in ziglang/zig Jun 16, 2022
The purpose of this branch is to switch to using an object file for each
independent function, in order to make linking simpler - instead of
relying on `-ffunction-sections` and `--gc-sections`, which involves the
linker doing the work of linking everything and then undoing work via
garbage collection, this will allow the linker to only include the
compilation units that are depended on in the first place.

This commit makes progress towards that goal.
nicholasbishop added a commit to nicholasbishop/rust that referenced this issue Nov 6, 2022
On arm, llvm treats the C calling convention as `aapcs` on soft-float
targets and `aapcs-vfp` on hard-float targets [1]. UEFI specifies in the
arm calling convention that floating point extensions aren't used [2],
so always translate `efiapi` to `aapcs` on arm.

[1]: rust-lang/compiler-builtins#116 (comment)
[2]: https://uefi.org/specs/UEFI/2.10/02_Overview.html#detailed-calling-convention

rust-lang#65815
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 10, 2022
…r=nagisa

Use aapcs for efiapi calling convention on arm

On arm, [llvm treats the C calling convention as `aapcs` on soft-float targets and `aapcs-vfp` on hard-float targets](rust-lang/compiler-builtins#116 (comment)). UEFI specifies in the arm calling convention that [floating point extensions aren't used](https://uefi.org/specs/UEFI/2.10/02_Overview.html#detailed-calling-convention), so always translate `efiapi` to `aapcs` on arm.

rust-lang#65815
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 10, 2022
…r=nagisa

Use aapcs for efiapi calling convention on arm

On arm, [llvm treats the C calling convention as `aapcs` on soft-float targets and `aapcs-vfp` on hard-float targets](rust-lang/compiler-builtins#116 (comment)). UEFI specifies in the arm calling convention that [floating point extensions aren't used](https://uefi.org/specs/UEFI/2.10/02_Overview.html#detailed-calling-convention), so always translate `efiapi` to `aapcs` on arm.

rust-lang#65815
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.

4 participants