Missed optimization because of missing inline attr on OsStr

[tesuji]

Godbolt link: https://rust.godbolt.org/z/bAU2UA
I expect these two snippets have the same optimized asm but they don't:

use std::ffi::OsStr;

pub fn foo(s: Option<&OsStr>) -> bool {
    s.map_or(false, |x| x == OsStr::new("so"))
}

use std::ffi::OsStr;

pub fn foo(s: Option<&OsStr>) -> bool {
    s == Some(OsStr::new("so"))
}

[nikic]

I think the bigger problem here is that the quite trivial AsRef and PartialEq implementations did not get inlined. I would not expect anything to optimize well without those getting inlined.

[michalt]

I'm not sure we can expect these two to give you the same assembly. They are actually a bit different -- the first one will construct OsStr conditionally (map_or will call the closure only for Some), whereas the second one will create the OsStr unconditionally before even checking if the parameter is Some or None.

As for the differences in the assembly wrt. comparisons themselves, this seems to be due to the difference between the code to check for equality of OsStr vs Option<&OsStr>.

I wonder, do you have a particular benchmark/workload where the first function is slower than expected?

[tesuji]

Still, if I change the first snippet like below, the asm is different too:

use std::ffi::OsStr;

pub fn foo(s: Option<&OsStr>) -> bool {
    let so = OsStr::new("so");
    s.map_or(false, |x| x == so)
}

[nagisa]

OsStr::new("so") is effectively a pointer cast, so it is warranted to expect assembly of at least similar quality.

Inlining does not happen because we do happen to have generated machine code for these trivial functions in libstd.rlib. Tacking some #[inline]s should suffice?

[tesuji]

After inlining some methods in #67169, I got the same asm between those two snippets:

check_stage1::foo:
 xor     eax, eax
 test    rdi, rdi
 je      .LBB0_5
 cmp     rsi, 2
 jne     .LBB0_5
 lea     rax, [rip, +, .Lanon.7bddf4f09674752ba4bdf737f126bcf4.0]
 cmp     rdi, rax
 je      .LBB0_3
 movzx   eax, word, ptr, [rdi]
 cmp     eax, 28531
 sete    al
.LBB0_5:
 ret
.LBB0_3:
 mov     al, 1
 ret