From e5789765606baebd983bd9b1c870cb8b57a0627b Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 15 Jun 2018 15:47:54 -0700 Subject: Store scalar pair bools as i8 in memory We represent `bool` as `i1` in a `ScalarPair`, unlike other aggregates, to optimize IR for checked operators and the like. With this patch, we still do so when the pair is an immediate value, but we use the `i8` memory type when the value is loaded or stored as an LLVM aggregate. So `(bool, bool)` looks like an `{ i1, i1 }` immediate, but `{ i8, i8 }` in memory. When a pair is a direct function argument, `PassMode::Pair`, it is still passed using the immediate `i1` type, but as a return value it will use the `i8` memory type. Also, `bool`-like` enum tags will now use scalar pairs when possible, where they were previously excluded due to optimization issues. --- src/test/codegen/function-arguments.rs | 2 +- src/test/codegen/scalar-pair-bool.rs | 40 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 src/test/codegen/scalar-pair-bool.rs (limited to 'src/test/codegen') diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index e3fa7a7db39..c027dece014 100644 --- a/src/test/codegen/function-arguments.rs +++ b/src/test/codegen/function-arguments.rs @@ -149,7 +149,7 @@ pub fn enum_id_1(x: Option>) -> Option> { x } -// CHECK: i16 @enum_id_2(i16) +// CHECK: { i8, i8 } @enum_id_2(i1 zeroext %x.0, i8 %x.1) #[no_mangle] pub fn enum_id_2(x: Option) -> Option { x diff --git a/src/test/codegen/scalar-pair-bool.rs b/src/test/codegen/scalar-pair-bool.rs new file mode 100644 index 00000000000..2078a245085 --- /dev/null +++ b/src/test/codegen/scalar-pair-bool.rs @@ -0,0 +1,40 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -O + +#![crate_type = "lib"] + +// CHECK: define { i8, i8 } @pair_bool_bool(i1 zeroext %pair.0, i1 zeroext %pair.1) +#[no_mangle] +pub fn pair_bool_bool(pair: (bool, bool)) -> (bool, bool) { + pair +} + +// CHECK: define { i8, i32 } @pair_bool_i32(i1 zeroext %pair.0, i32 %pair.1) +#[no_mangle] +pub fn pair_bool_i32(pair: (bool, i32)) -> (bool, i32) { + pair +} + +// CHECK: define { i32, i8 } @pair_i32_bool(i32 %pair.0, i1 zeroext %pair.1) +#[no_mangle] +pub fn pair_i32_bool(pair: (i32, bool)) -> (i32, bool) { + pair +} + +// CHECK: define { i8, i8 } @pair_and_or(i1 zeroext %arg0.0, i1 zeroext %arg0.1) +#[no_mangle] +pub fn pair_and_or((a, b): (bool, bool)) -> (bool, bool) { + // Make sure it can operate directly on the unpacked args + // CHECK: and i1 %arg0.0, %arg0.1 + // CHECK: or i1 %arg0.0, %arg0.1 + (a && b, a || b) +} -- cgit 1.4.1-3-g733a5 From 557736befc84fb72066128a8c42efe6e4e63a3b1 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 5 Jul 2018 14:22:09 -0700 Subject: Update scalar pairs per review comments --- src/librustc_codegen_llvm/mir/operand.rs | 6 +----- src/librustc_codegen_llvm/mir/place.rs | 6 +----- src/librustc_codegen_llvm/type_of.rs | 2 +- src/test/codegen/scalar-pair-bool.rs | 14 ++++++++++++++ 4 files changed, 17 insertions(+), 11 deletions(-) (limited to 'src/test/codegen') diff --git a/src/librustc_codegen_llvm/mir/operand.rs b/src/librustc_codegen_llvm/mir/operand.rs index 3069a155d1f..5d36eef99af 100644 --- a/src/librustc_codegen_llvm/mir/operand.rs +++ b/src/librustc_codegen_llvm/mir/operand.rs @@ -18,7 +18,7 @@ use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::sync::Lrc; use base; -use common::{self, CodegenCx, C_null, C_undef, C_usize}; +use common::{CodegenCx, C_null, C_undef, C_usize}; use builder::{Builder, MemFlags}; use value::Value; use type_of::LayoutLlvmExt; @@ -310,10 +310,6 @@ impl<'a, 'tcx> OperandValue { OperandValue::Pair(a, b) => { for (i, &x) in [a, b].iter().enumerate() { let llptr = bx.struct_gep(dest.llval, i as u64); - // Make sure to always store i1 as i8. - if common::val_ty(x) == Type::i1(bx.cx) { - assert_eq!(common::val_ty(llptr), Type::i8p(bx.cx)); - } let val = base::from_immediate(bx, x); bx.store_with_flags(val, llptr, dest.align, flags); } diff --git a/src/librustc_codegen_llvm/mir/place.rs b/src/librustc_codegen_llvm/mir/place.rs index 36dcd04b02e..e7f9457a6a1 100644 --- a/src/librustc_codegen_llvm/mir/place.rs +++ b/src/librustc_codegen_llvm/mir/place.rs @@ -16,7 +16,7 @@ use rustc::mir::tcx::PlaceTy; use rustc_data_structures::indexed_vec::Idx; use base; use builder::Builder; -use common::{CodegenCx, C_undef, C_usize, C_u8, C_u32, C_uint, C_null, C_uint_big, val_ty}; +use common::{CodegenCx, C_undef, C_usize, C_u8, C_u32, C_uint, C_null, C_uint_big}; use consts; use type_of::LayoutLlvmExt; use type_::Type; @@ -128,10 +128,6 @@ impl<'a, 'tcx> PlaceRef<'tcx> { } else if let layout::Abi::ScalarPair(ref a, ref b) = self.layout.abi { let load = |i, scalar: &layout::Scalar| { let llptr = bx.struct_gep(self.llval, i as u64); - // Make sure to always load i1 as i8. - if scalar.is_bool() { - assert_eq!(val_ty(llptr), Type::i8p(bx.cx)); - } let load = bx.load(llptr, self.align); scalar_load_metadata(load, scalar); if scalar.is_bool() { diff --git a/src/librustc_codegen_llvm/type_of.rs b/src/librustc_codegen_llvm/type_of.rs index 195f1c3b85a..0175d67803b 100644 --- a/src/librustc_codegen_llvm/type_of.rs +++ b/src/librustc_codegen_llvm/type_of.rs @@ -363,7 +363,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> { // Make sure to return the same type `immediate_llvm_type` would when // dealing with an immediate pair. This means that `(bool, bool)` is - // effectively represented as `{i8, i8}` in memory and `{i1, i1}` as an + // effectively represented as `{i8, i8}` in memory and two `i1`s as an // immediate, just like `bool` is typically `i8` in memory and only `i1` // when immediate. We need to load/store `bool` as `i8` to avoid // crippling LLVM optimizations or triggering other LLVM bugs with `i1`. diff --git a/src/test/codegen/scalar-pair-bool.rs b/src/test/codegen/scalar-pair-bool.rs index 2078a245085..f50e032f8e6 100644 --- a/src/test/codegen/scalar-pair-bool.rs +++ b/src/test/codegen/scalar-pair-bool.rs @@ -38,3 +38,17 @@ pub fn pair_and_or((a, b): (bool, bool)) -> (bool, bool) { // CHECK: or i1 %arg0.0, %arg0.1 (a && b, a || b) } + +// CHECK: define void @pair_branches(i1 zeroext %arg0.0, i1 zeroext %arg0.1) +#[no_mangle] +pub fn pair_branches((a, b): (bool, bool)) { + // Make sure it can branch directly on the unpacked bool args + // CHECK: br i1 %arg0.0 + if a { + println!("Hello!"); + } + // CHECK: br i1 %arg0.1 + if b { + println!("Goodbye!"); + } +} -- cgit 1.4.1-3-g733a5