[beta] librand unreachable pattern on armv7-unknown-linux-gnueabihf

[cuviper]

I just tried builds of 1.13.0-beta.3 on Fedora 24 and Rawhide (with LLVM 3.8 and 3.9 respectively), and both builds failed only on armv7hl, in the exact same place:

CFG_LLVM_LINKAGE_FILE=/builddir/build/BUILD/rustc-beta/armv7-unknown-linux-gnueabihf/rt/llvmdeps.rs LD_LIBRARY_PATH=/builddir/build/BUILD/rustc-beta/armv7-unknown-linux-gnueabihf/stage2/lib:/usr/lib:$LD_LIBRARY_PATH   armv7-unknown-linux-gnueabihf/stage2/bin/rustc --cfg stage2 -Clink-args=-Wl,-z,relro,-z,now -O --cfg rtopt -g  --target=armv7-unknown-linux-gnueabihf  -C prefer-dynamic -L "armv7-unknown-linux-gnueabihf/rt" -L native="/usr/lib"     --out-dir armv7-unknown-linux-gnueabihf/stage2/lib/rustlib/armv7-unknown-linux-gnueabihf/lib -C extra-filename=-f02899e6 -C metadata=f02899e6 src/librand/lib.rs
error[E0001]: unreachable pattern
  --> src/librand/distributions/gamma.rs:93:13
   |
93 |             0.0...1.0 => Small(GammaSmallShape::new_raw(shape, scale)),
   |             ^^^^^^^^^ this is an unreachable pattern

That full function is:

impl Gamma {
    /// Construct an object representing the `Gamma(shape, scale)`
    /// distribution.
    ///
    /// Panics if `shape <= 0` or `scale <= 0`.
    pub fn new(shape: f64, scale: f64) -> Gamma {
        assert!(shape > 0.0, "Gamma::new called with shape <= 0");
        assert!(scale > 0.0, "Gamma::new called with scale <= 0");

        let repr = match shape {
            1.0 => One(Exp::new(1.0 / scale)),
            0.0...1.0 => Small(GammaSmallShape::new_raw(shape, scale)),
            _ => Large(GammaLargeShape::new_raw(shape, scale)),
        };
        Gamma { repr: repr }
    }
}

This particular code hasn't changed in over a year, and my i686, x86_64, and aarch64 builds were fine with it. Only armv7hl has the error, and note even then it wasn't until --cfg stage2.

I had no problem with armv7hl on prior releases, so this appears to be a beta regression. I do have one patch applied from #36933 to disable NEON, but I also had that on 1.12.1 and it went fine. I'm baffled what else could be specific to armv7hl to cause this...

[TimNN]

This could be related to #37559 and it's fix (rust-lang/compiler-rt#25), now maybe the opposite situation (if the compiler uses some of the builtins during the unreachability checking).

cc @japaric

[TimNN]

@cuviper: Just to check:

• is armv7hl your host architecture?
• which compiler are you bootstrapping from?

[cuviper]

• Yes, Fedora builds always use native hosts.
• These are bootstrapping from Fedora's own 1.12.1.
  • I could start a build using upstream bootstrap binaries, if that's a useful data point

[TimNN]

Mh, no I think 1.12.1 was not affected by that bug, so using the upstream binaries wouldn't help, I think.

[TimNN]

If you want, you could try with rust-lang/compiler-rt@592717e reverted, although I'm not really sure how much that will help, the code responsible for pattern checking does not seem to need any builtins from a quick glance.

[cuviper]

I started a build with that reverted, if you care to watch paint dry...
http://koji.fedoraproject.org/koji/taskinfo?taskID=16341256

However, assuming your fix was correct, there are two more places with explicit "aapcs": __aeabi_cdcmpeq_check_nan and __aeabi_cfcmpeq_check_nan, used in support of the asm __aeabi_cdcmpeq and __aeabi_cfcmpeq. These sound like functions that could be used in pattern checking, but I don't know how you're checking this.

Of course the ABI of each check_nan needs to match what the asm caller expects. But this seems to open a big can of worms if all of those arm/*.S are using the wrong ABI, not matching their callers.

[TimNN]

However, assuming your fix was correct, there are two more places with explicit "aapcs":

Then that is probably the real problem.

but I don't know how you're checking this.

I just looked around a bit in check_match.rs, and found that from a glance, only <= was used on floats.

I expected the <= to not use any builtins since this is a hard float target (is it not?).

[cuviper]

Yes, it's hard float, but not NEON if that matters to your point.

Actually, I have another build going without that #36933 patch, in case NEON does make a difference.
http://koji.fedoraproject.org/koji/taskinfo?taskID=16340627

[TimNN]

Alright, so parsing floats seems to be broken (printing constants works fine):

Test program:

use std::io::{self, BufRead};

fn main() {
    let inp = io::stdin();
    let mut inp = inp.lock();

    let mut line = String::new();

    inp.read_line(&mut line).unwrap();

    println!("{}", line.trim().parse::<f64>().unwrap());
}

For input 1, the output is 0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005.

So the comparisons / match checking are probably fine but the parsed floats are bad.

Edit: This was compiled with the official rust beta binaries.

cc @alexcrichton

[TimNN]

This affects nightly-2016-11-06 but not nightly-2016-11-03 (Changes), notably that change set does not include rust-lang/compiler-rt#25.

Edit: forget that, I must have confused something, I cannot reproduce on either of those nightlies.

[cuviper]

The NEON build (withholding #36933) still failed in the same way.

The build with reverted rust-lang/compiler-rt@592717e is still going, but it just got past that failure point of librand --cfg stage2, so it seems in good shape so far.

[TimNN]

Alright, so with the test program above, the current beta, running on an RPi 2 with armv7-unknown-linux-gnu, I observer the following:

• the parsing code calls __floatundidf
• __floatundidf returns the (correct) value in d0;
  the callee of __floatundidf expects the result in r0, r1
• the callee is RawFloat::from_int, which is just an x: u64 as f64

[japaric]

So, this is ... interesting.

According to this bug, for eabihf ("hard float") target, libgcc calls floating point intrinsics, like "floatundidf", using the hard float convention (argument passing over FPU registers (s0, d0, etc.)) (FWIW, this seems consistent to me, it respects the global calling convention, the "hf" in gnueabihf) whereas clang/llvm calls them using the soft float convention (argument passing over "general purpose" registers (r0, r1)). It seems that llvm does that because libgcc used to do it that way but libgcc (apparently) broke its ABI and changed from soft float to the current hard float ABI.

So, what to do. For gnueabihf, we should compile these floating point intrinsics using the soft float convention because that's how LLVM call these intrinsics. Except for the powisf2 intrinsic, that should be compiled using the hard float convention to avoid re-introducing #37559.

It's weird that LLVM calls some floating point intrinsics using the soft float convention but others with the hard float one ... but we have to do what LLVM dictates.