Dellvmize some intrinsics (use u32 instead of Self in some integer intrinsics)

[WaffleLapkin]

This implements rust-lang/compiler-team#693 minus what was implemented in #123226.

Note: I decided to not change shl/... builder methods, as it just doesn't seem worth it.

r? @scottmcm

[rustbot]

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

[RalfJung]

This looks like it can ICE when bswap/bitrevere is called with a too small ret_layout.

[WaffleLapkin]

How would that happen though? The intrinsics are defined like pub fn bitreverse<T: Copy>(x: T) -> T;, so the ret_layout must be the same?

[RalfJung]

That's an extremely non-local invariant. This function here should make sense on its own without worrying about what happens in the standard library.

[RalfJung]

Maybe add an assertion that for bswap/bitreverse, the two layouts are the same.

[WaffleLapkin]

I think this is true in general with intrinsics, we implicitly depend on their signature in a lot of ways. Either way, I added an assertion.

[RalfJung]

In the interpreter and Miri we always have local assertions for these things.

[scottmcm]

(Looks like some CI issues?)

[WaffleLapkin]

#[cfg(not(bootstrap))] makes tidy think unsafe is not documented. fun.

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

[rustbot]

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

[rustbot]

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

[WaffleLapkin]

@rustbot ready

I was not able to fix the gcc backend (could not run tests locally and using CI seemed like too much trouble), so I just commented the tests out ^^'

Other backends should be working, unless I missed something.

[antoyo]

Is the error you got from the GCC codegen the one we see in the message above?
This one:

##[error]   --> example/example.rs:157:5
    |
    |
156 | unsafe fn use_ctlz_nonzero(a: u16) -> u16 {
    |                                       --- expected `u16` because of return type
157 |     intrinsics::ctlz_nonzero(a)
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u16`, found `u32`
    |
help: you can convert a `u32` to a `u16` and panic if the converted value doesn't fit
    |
157 |     intrinsics::ctlz_nonzero(a).try_into().unwrap()

?
Because that's a frontend error, so that seems weird to me especially since you changed the signature here.

[WaffleLapkin]

@antoyo this is the error I got, but if I'd fix it, I would just get a different error, because gcc (now) casts to the wrong type

[antoyo]

Did you try to change arg_type to self.u32_type in this code?

[WaffleLapkin]

I did not, precisely because I wasn't able to test things

[antoyo]

Ok, you might want to join this discussion so we can figure out how to make this better.

[scottmcm]

The changes here all look good to me.

@RalfJung , did you want to look further at the CTFE parts?

I'm a bit uncertain about the cg_gcc state here. The amount being disabled is small so seems fine enough to do and resolve later, but I also don't feel confident in saying what the expectations are for it.

[RalfJung]

The interpreter changes LGTM.

[antoyo]

I'm a bit uncertain about the cg_gcc state here. The amount being disabled is small so seems fine enough to do and resolve later, but I also don't feel confident in saying what the expectations are for it.

While we do prefer to avoid disabling code like was done here (and we're always happy to help implement the stuff in the PR), we're OK with people not implementing the stuff and doing it on our side.

I guess we could think at some point of allowing us to fix the cg_gcc stuff in PR made by other people, but we should also think about that carefully to not block people.

All of that to say, I'm OK with the cg_gcc changes here.

[workingjubilee]

hokay!

@bors r=scottmcm,RalfJung,antoyo

[bors]

📌 Commit 468179c has been approved by scottmcm,RalfJung,antoyo

It is now in the queue for this repository.

[WaffleLapkin]

Thanks to everyone involved <3