Invalid monomorphization when -C link-dead-code is used

[xd009642]

I've tried to compile this crate https://github.com/yoanlcq/vek as part of running code coverage and with RUSTFLAGS="-C link-dead-code" and --features platform_intrinsics it fails to build. But without -C link-dead-code it builds and runs fine which is what I'd expect with the flag present.

Sample error (they're all the same error just different bits of code:

error[E0511]: invalid monomorphization of `simd_reduce_any` intrinsic: unsupported simd_reduce_any from `vec::repr_simd::extent2::Extent2<bool>` with element `bool` to `bool`
    --> src/vec.rs:1346:43
     |
1346 |                       simd_llvm => unsafe { simd_llvm::simd_reduce_any(self) },
     |                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
3195 | /     vec_impl_all_vecs!{
3196 | |         simd
3197 | |         #[repr(simd)]
3198 | |     }
     | |_____- in this macro invocation
     |
     = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 14 previous errors

[camelid]

(Not sure if I-monomorphization is correct, remove it if it's not.)

[xMAC94x]

Any progress on this error ? There are many projects which are having problems, as they use vek in one of their dependencies

[tesuji]

Since this crate cannot be built with --features platform_intrinsics on stable:

   Compiling approx v0.3.2
error[E0658]: platform intrinsics are experimental and possibly buggy
  --> src/simd_llvm.rs:13:8
   |
13 | extern "platform-intrinsic" {
   |        ^^^^^^^^^^^^^^^^^^^^
   |
   = note: see issue #27731 <https://github.com/rust-lang/rust/issues/27731> for more information

warning: unused import: `crate::simd_llvm`
  --> src/vec.rs:21:5
   |
21 | use crate::simd_llvm;
   |     ^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0658`.

@rustbot modify labels: requires-nightly

[jyn514]

@xd009642 do you know if this used to work on an old version of rust? Or has it always been an issue?

[tesuji]

MCVE: https://rust.godbolt.org/z/ax56Mj

// compile-flags: --crate-type=rlib -C link-dead-code
#![feature(platform_intrinsics)]
#![feature(repr_simd)]

extern "platform-intrinsic" {
    pub fn simd_reduce_all<T>(x: T) -> bool;
}

#[repr(simd)]
// #[repr(C)]
pub struct Vec3<T> {
    pub x: T,
    pub y: T,
    pub z: T,
}

impl Vec3<bool> {
    #[inline]
    pub fn reduce_and(self) -> bool {
        unsafe { simd_reduce_all(self) }
    }
}

[xMAC94x]

I used the MCVE and checked it with those toolchains on linux. And all show exact the same error.

• nightly-2020-10-15
• nightly-2020-10-10
• nightly-2020-09-15
• nightly-2020-08-15
• nightly-2020-06-15
• nightly-2020-04-15
• nightly-2020-02-15
• nightly-2019-12-15
• nightly-2020-10-15
• nightly-2019-08-15
• nightly-2018-08-15
• nightly-2017-08-15

(long version is always: nightly-20xx-yy-zz-x86_64-unknown-linux-gnu)
so i would guess that this code has never worked

[xd009642]

@jyn514 I've no idea when it started, it's not my code I just write a tool that adds the linker flag 😅

[tesuji]

I believe the code should never be compiled. Because if you manually replace Vec3<bool> with a struct
of 3 members of type bool, repr(simd) will error.
https://rust.godbolt.org/z/KG5fKT

#![feature(repr_simd)]

#[repr(simd)]
pub struct Vec3 { // error[E0077]: SIMD vector element type should be machine type
    pub x: bool,
    pub y: bool,
    pub z: bool,
}

In short, an useful error message is lost, instead a less useful error message replaces it.

[apiraino]

Assigning P-medium as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

[workingjubilee]

Unfortunately, because this uses T generically we allow type-checking to pass this by inside rustc_typeck, as it may be a valid SIMD type and I am not aware of a way to determine this before monomorphization time. It is not clear what we can do here, aside from replace the need for this kind of code entirely via a stable and typesafe portable SIMD API.

[camelid]

Unfortunately, because this uses T generically we allow type-checking to pass this by inside rustc_typeck, as it may be a valid SIMD type and I am not aware of a way to determine this before monomorphization time.

Could some code be added to the monomorphizer that replaces the less useful error with a more useful error like the one reported by rustc_typeck in non-generic contexts?

[workingjubilee]

Perhaps, but this only occurs because -Clink-dead-code is used. So we would also want to address the issues raised in other -Clink-dead-code issues, such as

• link-dead-code limitations (bad for code coverage) #39293
• Linker error because -wL as-needed conflicts with -C link-dead-code #64685
• rustc -Clink-dead-code causes MSVC linker to produce invalid binaries when LLVM InstrProf counters are enabled #76038

I have a hunch that says that unconstrained linking-in of code that was intended to be omitted may simply be a bad idea, though. There is a need served by it, but it is not clear that that need is best addressed in this way. In this case, however, we can simply eliminate the need directly, however, by following through with #63633, which is what I have been working on.

[camelid]

(meta: It might be good to rename the link-dead-code label to A-link-dead-code for consistency with other labels. And turn it yellow, like the other A- labels.)

[workingjubilee]

It's a little more like the F- tags notionally, at the moment, so I didn't seek immediate consistency with A-. Aside from that note, you are welcome to evolve it in whatever direction seems to fit the repo's aesthetic.

[camelid]

My understanding is that F- labels are more reserved for feature gate names, and A- tags refer to a particular area of the compiler/stdlib/etc. So, e.g., A-code-coverage refers to -Z instrument-coverage, whereas F-generic_associated_types refers to #![feature(generic_associated_types)].