search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.58k stars 12.62k forks source link

Invalid monomorphization when `-C link-dead-code` is used #77529

Open xd009642 opened 4 years ago

xd009642 commented 4 years ago

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 commented 4 years ago

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

xMAC94x commented 3 years ago

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

tesuji commented 3 years ago

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 commented 3 years ago

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

tesuji commented 3 years ago

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 commented 3 years ago

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

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

xd009642 commented 3 years ago

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

tesuji commented 3 years ago

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 commented 3 years ago

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

workingjubilee commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

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 https://github.com/rust-lang/rust/issues/63633, which is what I have been working on.

camelid commented 3 years ago

(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 commented 3 years ago

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 commented 3 years ago

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)].