Skip to content

rustc_codegen_ssa: use bitcasts instead of type punning for scalar transmutes. #79801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1395,6 +1395,25 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
dst: PlaceRef<'tcx, Bx::Value>,
) {
let src = self.codegen_operand(bx, src);

// Special-case transmutes between scalars as simple bitcasts.
match (&src.layout.abi, &dst.layout.abi) {
(abi::Abi::Scalar(src_scalar), abi::Abi::Scalar(dst_scalar)) => {
// HACK(eddyb) LLVM doesn't like `bitcast`s between pointers and non-pointers.
if (src_scalar.value == abi::Pointer) == (dst_scalar.value == abi::Pointer) {
assert_eq!(src.layout.size, dst.layout.size);

// NOTE(eddyb) the `from_immediate` and `to_immediate_scalar`
// conversions allow handling `bool`s the same as `u8`s.
let src = bx.from_immediate(src.immediate());
let src_as_dst = bx.bitcast(src, bx.backend_type(dst.layout));
Immediate(bx.to_immediate_scalar(src_as_dst, dst_scalar)).store(bx, dst);
return;
}
}
_ => {}
}

let llty = bx.backend_type(src.layout);
let cast_ptr = bx.pointercast(dst.llval, bx.type_ptr_to(llty));
let align = src.layout.align.abi.min(dst.align);
Expand Down
85 changes: 85 additions & 0 deletions src/test/codegen/transmute-scalar.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// compile-flags: -O -C no-prepopulate-passes

#![crate_type = "lib"]

// FIXME(eddyb) all of these tests show memory stores and loads, even after a
// scalar `bitcast`, more special-casing is required to remove `alloca` usage.

// CHECK: define i32 @f32_to_bits(float %x)
// CHECK: %2 = bitcast float %x to i32
// CHECK-NEXT: store i32 %2, i32* %0
// CHECK-NEXT: %3 = load i32, i32* %0
// CHECK: ret i32 %3
#[no_mangle]
pub fn f32_to_bits(x: f32) -> u32 {
unsafe { std::mem::transmute(x) }
}

// CHECK: define i8 @bool_to_byte(i1 zeroext %b)
// CHECK: %1 = zext i1 %b to i8
// CHECK-NEXT: store i8 %1, i8* %0
// CHECK-NEXT: %2 = load i8, i8* %0
// CHECK: ret i8 %2
#[no_mangle]
pub fn bool_to_byte(b: bool) -> u8 {
unsafe { std::mem::transmute(b) }
}

// CHECK: define zeroext i1 @byte_to_bool(i8 %byte)
// CHECK: %1 = trunc i8 %byte to i1
// CHECK-NEXT: %2 = zext i1 %1 to i8
// CHECK-NEXT: store i8 %2, i8* %0
// CHECK-NEXT: %3 = load i8, i8* %0
// CHECK-NEXT: %4 = trunc i8 %3 to i1
// CHECK: ret i1 %4
#[no_mangle]
pub unsafe fn byte_to_bool(byte: u8) -> bool {
std::mem::transmute(byte)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add special test around the pointer special case? (ptr->ptr, ptr->int, ...) I don't anticipate this PR breaking it, but it would be good to have it to be sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, added ptr->ptr, ptr->int, int->ptr, and included comments with what they might become in the future.


// CHECK: define i8* @ptr_to_ptr(i16* %p)
// CHECK: %2 = bitcast i16* %p to i8*
// CHECK-NEXT: store i8* %2, i8** %0
// CHECK-NEXT: %3 = load i8*, i8** %0
// CHECK: ret i8* %3
#[no_mangle]
pub fn ptr_to_ptr(p: *mut u16) -> *mut u8 {
unsafe { std::mem::transmute(p) }
}

// HACK(eddyb) scalar `transmute`s between pointers and non-pointers are
// currently not special-cased like other scalar `transmute`s, because
// LLVM requires specifically `ptrtoint`/`inttoptr` instead of `bitcast`.
//
// Tests below show the non-special-cased behavior (with the possible
// future special-cased instructions in the "NOTE(eddyb)" comments).

// CHECK: define [[USIZE:i[0-9]+]] @ptr_to_int(i16* %p)

// NOTE(eddyb) see above, the following two CHECK lines should ideally be this:
// %2 = ptrtoint i16* %p to [[USIZE]]
// store [[USIZE]] %2, [[USIZE]]* %0
// CHECK: %2 = bitcast [[USIZE]]* %0 to i16**
// CHECK-NEXT: store i16* %p, i16** %2

// CHECK-NEXT: %3 = load [[USIZE]], [[USIZE]]* %0
// CHECK: ret [[USIZE]] %3
#[no_mangle]
pub fn ptr_to_int(p: *mut u16) -> usize {
unsafe { std::mem::transmute(p) }
}

// CHECK: define i16* @int_to_ptr([[USIZE]] %i)

// NOTE(eddyb) see above, the following two CHECK lines should ideally be this:
// %2 = inttoptr [[USIZE]] %i to i16*
// store i16* %2, i16** %0
// CHECK: %2 = bitcast i16** %0 to [[USIZE]]*
// CHECK-NEXT: store [[USIZE]] %i, [[USIZE]]* %2

// CHECK-NEXT: %3 = load i16*, i16** %0
// CHECK: ret i16* %3
#[no_mangle]
pub fn int_to_ptr(i: usize) -> *mut u16 {
unsafe { std::mem::transmute(i) }
}