make memcmp return a value of c_int_width instead of i32

[drmorr0]

This is an attempt to fix #32610 and #78022, namely, that memcmp always returns an i32 regardless of the platform. I'm running into some issues and was hoping I could get some help.

Here's what I've been attempting so far:

1. Build the stage0 compiler with all the changes expect for the changes in library/core/src/slice/cmp.rs and compiler/rustc_codegen_llvm/src/context.rs; this is because target_c_int_width isn't passed through and recognized as a valid config option yet. I'm building with ./x.py build --stage 0 library/core library/proc_macro compiler/rustc
2. Next I add in the #[cfg(c_int_width = ...)] params to cmp.rs and context.rs and build the stage 1 compiler by running ./x.py build --keep-stage 0 --stage 1 library/core library/proc_macro compiler/rustc. This step now runs successfully.
3. Lastly, I try to build the test program for AVR mentioned in Array comparisons don't work on AVR #78022 with RUSTFLAGS="--emit llvm-ir" cargo build --release, and look at the resulting llvm IR, which still shows:

...
%11 = call addrspace(1) i32 @memcmp(i8* nonnull %5, i8* nonnull %10, i16 5) #7, !dbg !1191                                                                                                                                                                                                                                %.not = icmp eq i32 %11, 0, !dbg !1191
...
; Function Attrs: nounwind optsize                                                                                                                                                                                                                                                                                          declare i32 @memcmp(i8*, i8*, i16) local_unnamed_addr addrspace(1) #4

Any ideas what I'm missing here? Alternately, if this is totally the wrong approach I'm open to other suggestions.

cc @Rahix

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon.

Please see the contribution instructions for more information.

[drmorr0]

Ok, thanks to your pointers I got this actually working and confirmed that this works for the test program.

I think I need to do this in two parts to pass CI? The first part will make the compiler handle different c_int_widths, and then I can uncomment the #[cfg(...)] bits in core.

Also, I'm unsure what to do for the "default" case for the match statement... currently this outputs the following if you use a bad value for target-c-int-width:

error: internal compiler error: compiler/rustc_codegen_llvm/src/context.rs:784:18: Unsupported target_c_int_width = 17
thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1147:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md
note: rustc 1.58.0-dev running on x86_64-unknown-linux-gnu
note: compiler flags: -Z unstable-options -C opt-level=s -C panic=abort -C linker-plugin-lto -C codegen-units=1 -C debuginfo=2 --crate-type lib
note: some of the compiler flags provided by cargo are hidden

Lastly, are there any tests I should write, or any other things I can do to help get this through?

[cuviper]

I think I need to do this in two parts to pass CI? The first part will make the compiler handle different c_int_widths, and then I can uncomment the #[cfg(...)] bits in core.

You can use cfg(bootstrap) to work with the stage0 compiler that doesn't have your changes yet.
e.g. combined: #[cfg(any(bootstrap, c_int_width = "32"))]

[drmorr0]

You can use cfg(bootstrap) to work with the stage0 compiler that doesn't have your changes yet.
e.g. combined: #[cfg(any(bootstrap, c_int_width = "32"))]

Fixed, thanks!

[drmorr0]

@joshtriplett Hi, just checking if there's anything else I need to do on this to get it merged in?

[joshtriplett]

r? @rust-lang/compiler

[petrochenkov]

The codegen changes look reasonable to me, although I'm not an expert in that area.

[mrk-its]

I'm testing these changes in my rust fork for mos arch (8bit 6502 CPU) and everything works fine. https://github.com/mrk-its/rust/tree/mos_target

[nagisa]

I'm going to request that Vadim's review comments are addressed. I don't believe this PR should land as is.

[Dylan-DPC]

@drmorr0 any updates on this?

[drmorr0]

@Dylan-DPC I have not had a chance to revisit this yet, I'm hoping to get to it soon-ish, though.

[drmorr0]

Ok I think this is ready; I'm trying to get my test environment back up so that I can confirm it's generating the right thing but some other stuff has broken in the interim.

[petrochenkov]

Thanks!
@bors r+

[bors]

📌 Commit aa67016 has been approved by petrochenkov

[bors]

⌛ Testing commit aa67016 with merge 15a242a...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 15a242a to master...

[rust-timer]

Finished benchmarking commit (15a242a): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression