Skip to content

Commit

Permalink
Auto merge of rust-lang#90791 - drmorr0:drmorr-memcmp-cint-cfg, r=pet…
Browse files Browse the repository at this point in the history
…rochenkov

make memcmp return a value of c_int_width instead of i32

This is an attempt to fix rust-lang#32610 and rust-lang#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 rust-lang#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`
  • Loading branch information
bors committed Apr 3, 2022
2 parents ec7b753 + aa67016 commit 15a242a
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 4 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_gcc/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ impl<'gcc, 'tcx> ConstMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
self.const_uint(self.type_i1(), val as u64)
}

fn const_i16(&self, i: i16) -> RValue<'gcc> {
self.const_int(self.type_i16(), i as i64)
}

fn const_i32(&self, i: i32) -> RValue<'gcc> {
self.const_int(self.type_i32(), i as i64)
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
self.const_uint(self.type_i1(), val as u64)
}

fn const_i16(&self, i: i16) -> &'ll Value {
self.const_int(self.type_i16(), i as i64)
}

fn const_i32(&self, i: i32) -> &'ll Value {
self.const_int(self.type_i32(), i as i64)
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_codegen_llvm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,10 @@ impl<'ll> CodegenCx<'ll, '_> {

// This isn't an "LLVM intrinsic", but LLVM's optimization passes
// recognize it like one and we assume it exists in `core::slice::cmp`
ifn!("memcmp", fn(i8p, i8p, t_isize) -> t_i32);
match self.sess().target.arch.as_str() {
"avr" | "msp430" => ifn!("memcmp", fn(i8p, i8p, t_isize) -> t_i16),
_ => ifn!("memcmp", fn(i8p, i8p, t_isize) -> t_i32),
}

// variadic intrinsics
ifn!("llvm.va_start", fn(i8p) -> void);
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,10 @@ impl<'ll, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'_, 'll, 'tcx> {
let b_ptr = self.bitcast(b, i8p_ty);
let n = self.const_usize(layout.size().bytes());
let cmp = self.call_intrinsic("memcmp", &[a_ptr, b_ptr, n]);
self.icmp(IntPredicate::IntEQ, cmp, self.const_i32(0))
match self.cx.sess().target.arch.as_str() {
"avr" | "msp430" => self.icmp(IntPredicate::IntEQ, cmp, self.const_i16(0)),
_ => self.icmp(IntPredicate::IntEQ, cmp, self.const_i32(0)),
}
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/traits/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub trait ConstMethods<'tcx>: BackendTypes {
fn const_uint(&self, t: Self::Type, i: u64) -> Self::Value;
fn const_uint_big(&self, t: Self::Type, u: u128) -> Self::Value;
fn const_bool(&self, val: bool) -> Self::Value;
fn const_i16(&self, i: i16) -> Self::Value;
fn const_i32(&self, i: i32) -> Self::Value;
fn const_u32(&self, i: u32) -> Self::Value;
fn const_u64(&self, i: u64) -> Self::Value;
Expand Down
3 changes: 3 additions & 0 deletions library/core/src/ffi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ type_alias! { "c_schar.md", c_schar = i8, NonZero_c_schar = NonZeroI8; }
type_alias! { "c_uchar.md", c_uchar = u8, NonZero_c_uchar = NonZeroU8; }
type_alias! { "c_short.md", c_short = i16, NonZero_c_short = NonZeroI16; }
type_alias! { "c_ushort.md", c_ushort = u16, NonZero_c_ushort = NonZeroU16; }
#[cfg(any(target_arch = "avr", target_arch = "msp430"))]
type_alias! { "c_int.md", c_int = i16, NonZero_c_int = NonZeroI16; }
#[cfg(not(any(target_arch = "avr", target_arch = "msp430")))]
type_alias! { "c_int.md", c_int = i32, NonZero_c_int = NonZeroI32; }
type_alias! { "c_uint.md", c_uint = u32, NonZero_c_uint = NonZeroU32; }
type_alias! { "c_long.md", c_long = i32, NonZero_c_long = NonZeroI32;
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/slice/cmp.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Comparison traits for `[T]`.

use crate::cmp::{self, Ordering};
use crate::ffi;
use crate::mem;

use super::from_raw_parts;
Expand All @@ -13,8 +14,7 @@ extern "C" {
///
/// Returns 0 for equal, < 0 for less than and > 0 for greater
/// than.
// FIXME(#32610): Return type should be c_int
fn memcmp(s1: *const u8, s2: *const u8, n: usize) -> i32;
fn memcmp(s1: *const u8, s2: *const u8, n: usize) -> ffi::c_int;
}

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down

0 comments on commit 15a242a

Please sign in to comment.