Add missing no_panic and inline annotations

[gnzlbg]

No description provided.

[gnzlbg]

The failure is expected, the bugs in these APIs haven't been fixed yet.

[gnzlbg]

This is ready, no public APIs should have panic code in release mode after this.

[alexcrichton]

Why is this adding #[inline] annotations to so many functions? It doesn't actually look like anything here was fixed, it's just relying on aggressive inlining?

[gnzlbg]

Why is this adding #[inline] annotations to so many functions?

All public APIs that aren't #[inline] cannot be inlined across crates unless -C lto is different from off, so the first commit of this PR makes the public APIs that were not inline, #[inline].

It doesn't actually look like anything here was fixed, it's just relying on aggressive inlining?

The compiler was generating panic handling code for some of these functions in optimized builds, which is what no_panic catches. Inlining some of the internal APIs fixes that. I did not have to add these #[inline] attributes on #198, probably because making these functions extern "C" fixes that as well.

[alexcrichton]

But yes that's the point of a crate, to actually compile code so we don't always force all users to compile inlined code. Why would every single function here need to be #[inline]? None of these functions are LTO'd with anything else anyway since they're rarely included in binaries directly. If they are then it seems like it should be up to the user to pass -C lto to actually LTO things rather than forcing LTO to always happen.

Inlining some of the internal APIs fixes that.

That seems like there's still a bug with panics then, and we're just papering over that with enough inlining.

probably because making these functions extern "C" fixes that as well.

That's not really fixing it I think but again just papering over it. It tweaks codegen enough that landing pads aren't always necessary here and there.

[gnzlbg]

Why would every single function here need to be #[inline]? None of these functions are LTO'd with anything else anyway since they're rarely included in binaries directly.

I believed compiler-builtins is always statically linked into the binary. Isn't this the case ?

[alexcrichton]

Sorry yes what I mean with included there is via extern crate such that it's actually a candidate for LTO. In essence none of these functions are ever considered candidates for LTO, and they can't really due to how LLVM works.

[gnzlbg]

In essence none of these functions are ever considered candidates for LTO, and they can't really due to how LLVM works.

Oh, I thought LLVM would just emit symbols for these, and then at link time, if those symbols have LTO information, LLVM would be able to optimize through these. Obviously, this never happens when liking against the system's libm, but I am not sure I follow why this can't happen when compiling a pure-Rust libm with LTO information and linking it to the final binary via compiler-builtins. AFAICT the compiler-builtin "symbols" are just normal symbols once LTO starts.

[gnzlbg]

So I'm closing this, I'm going to try to add no_panic to all methods where we can add them, and fill issues for those were we cannot. In those methods, something fishy will be going on. Whether we temporarily fix that by locally adding #[inline], or whether we manage to find the root of the issues quickly, we will see. I'd prefer not to use #[inline].