Skip to content

Commit cca2817

Browse files
committed
Auto merge of #26198 - stygstra:issue-24258, r=huonw
When overflow checking on `<<` and `>>` was added for integers, the `<<` and `>>` operations broke for SIMD types (`u32x4`, `i16x8`, etc.). This PR implements checked shifts on SIMD types. Fixes #24258.
2 parents 306a99e + 875f50a commit cca2817

11 files changed

+468
-11
lines changed

src/librustc_trans/trans/expr.rs

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ pub use self::Dest::*;
5252
use self::lazy_binop_ty::*;
5353

5454
use back::abi;
55-
use llvm::{self, ValueRef};
55+
use llvm::{self, ValueRef, TypeKind};
5656
use middle::check_const;
5757
use middle::def;
5858
use middle::lang_items::CoerceUnsizedTraitLangItem;
@@ -2458,12 +2458,10 @@ impl OverflowOpViaInputCheck {
24582458
// (since that is where the 32/64 distinction is relevant) but
24592459
// the mask's type must match the RHS type (since they will
24602460
// both be fed into a and-binop)
2461-
let invert_mask = !shift_mask_val(lhs_llty);
2462-
let invert_mask = C_integral(rhs_llty, invert_mask, true);
2461+
let invert_mask = shift_mask_val(bcx, lhs_llty, rhs_llty, true);
24632462

24642463
let outer_bits = And(bcx, rhs, invert_mask, binop_debug_loc);
2465-
let cond = ICmp(bcx, llvm::IntNE, outer_bits,
2466-
C_integral(rhs_llty, 0, false), binop_debug_loc);
2464+
let cond = build_nonzero_check(bcx, outer_bits, binop_debug_loc);
24672465
let result = match *self {
24682466
OverflowOpViaInputCheck::Shl =>
24692467
build_unchecked_lshift(bcx, lhs, rhs, binop_debug_loc),
@@ -2479,9 +2477,46 @@ impl OverflowOpViaInputCheck {
24792477
}
24802478
}
24812479

2482-
fn shift_mask_val(llty: Type) -> u64 {
2483-
// i8/u8 can shift by at most 7, i16/u16 by at most 15, etc.
2484-
llty.int_width() - 1
2480+
fn shift_mask_val<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
2481+
llty: Type,
2482+
mask_llty: Type,
2483+
invert: bool) -> ValueRef {
2484+
let kind = llty.kind();
2485+
match kind {
2486+
TypeKind::Integer => {
2487+
// i8/u8 can shift by at most 7, i16/u16 by at most 15, etc.
2488+
let val = llty.int_width() - 1;
2489+
if invert {
2490+
C_integral(mask_llty, !val, true)
2491+
} else {
2492+
C_integral(mask_llty, val, false)
2493+
}
2494+
},
2495+
TypeKind::Vector => {
2496+
let mask = shift_mask_val(bcx, llty.element_type(), mask_llty.element_type(), invert);
2497+
VectorSplat(bcx, mask_llty.vector_length(), mask)
2498+
},
2499+
_ => panic!("shift_mask_val: expected Integer or Vector, found {:?}", kind),
2500+
}
2501+
}
2502+
2503+
// Check if an integer or vector contains a nonzero element.
2504+
fn build_nonzero_check<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
2505+
value: ValueRef,
2506+
binop_debug_loc: DebugLoc) -> ValueRef {
2507+
let llty = val_ty(value);
2508+
let kind = llty.kind();
2509+
match kind {
2510+
TypeKind::Integer => ICmp(bcx, llvm::IntNE, value, C_null(llty), binop_debug_loc),
2511+
TypeKind::Vector => {
2512+
// Check if any elements of the vector are nonzero by treating
2513+
// it as a wide integer and checking if the integer is nonzero.
2514+
let width = llty.vector_length() as u64 * llty.element_type().int_width();
2515+
let int_value = BitCast(bcx, value, Type::ix(bcx.ccx(), width));
2516+
build_nonzero_check(bcx, int_value, binop_debug_loc)
2517+
},
2518+
_ => panic!("build_nonzero_check: expected Integer or Vector, found {:?}", kind),
2519+
}
24852520
}
24862521

24872522
// To avoid UB from LLVM, these two functions mask RHS with an
@@ -2507,7 +2542,14 @@ fn build_unchecked_rshift<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
25072542
let rhs = base::cast_shift_expr_rhs(bcx, ast::BinOp_::BiShr, lhs, rhs);
25082543
// #1877, #10183: Ensure that input is always valid
25092544
let rhs = shift_mask_rhs(bcx, rhs, binop_debug_loc);
2510-
let is_signed = ty::type_is_signed(lhs_t);
2545+
let tcx = bcx.tcx();
2546+
let is_simd = ty::type_is_simd(tcx, lhs_t);
2547+
let intype = if is_simd {
2548+
ty::simd_type(tcx, lhs_t)
2549+
} else {
2550+
lhs_t
2551+
};
2552+
let is_signed = ty::type_is_signed(intype);
25112553
if is_signed {
25122554
AShr(bcx, lhs, rhs, binop_debug_loc)
25132555
} else {
@@ -2519,8 +2561,7 @@ fn shift_mask_rhs<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
25192561
rhs: ValueRef,
25202562
debug_loc: DebugLoc) -> ValueRef {
25212563
let rhs_llty = val_ty(rhs);
2522-
let mask = shift_mask_val(rhs_llty);
2523-
And(bcx, rhs, C_integral(rhs_llty, mask, false), debug_loc)
2564+
And(bcx, rhs, shift_mask_val(bcx, rhs_llty, rhs_llty, false), debug_loc)
25242565
}
25252566

25262567
fn with_overflow_check<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, oop: OverflowOp, info: NodeIdAndSpan,
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
12+
// compile-flags: -C debug-assertions
13+
14+
#![feature(core_simd)]
15+
16+
use std::simd::i32x4;
17+
18+
// (Work around constant-evaluation)
19+
fn id<T>(x: T) -> T { x }
20+
21+
fn main() {
22+
let _x = i32x4(1, 0, 0, 0) << id(i32x4(32, 0, 0, 0));
23+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
12+
// compile-flags: -C debug-assertions
13+
14+
#![feature(core_simd)]
15+
16+
use std::simd::i32x4;
17+
18+
// (Work around constant-evaluation)
19+
fn id<T>(x: T) -> T { x }
20+
21+
fn main() {
22+
let _x = i32x4(1, 0, 0, 0) << id(i32x4(-1, 0, 0, 0));
23+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
12+
// compile-flags: -C debug-assertions
13+
14+
#![feature(core_simd)]
15+
16+
use std::simd::u64x2;
17+
18+
// (Work around constant-evaluation)
19+
fn id<T>(x: T) -> T { x }
20+
21+
fn main() {
22+
let _x = u64x2(1, 0) << id(u64x2(64, 0));
23+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
12+
// compile-flags: -C debug-assertions
13+
14+
// This function is checking that our automatic truncation does not
15+
// sidestep the overflow checking.
16+
17+
#![feature(core_simd)]
18+
19+
use std::simd::i8x16;
20+
21+
fn eq_i8x16(i8x16(x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12, x13, x14, x15): i8x16,
22+
i8x16(y0, y1, y2, y3, y4, y5, y6, y7, y8, y9, y10, y11, y12, y13, y14, y15): i8x16)
23+
-> bool
24+
{
25+
(x0 == y0) && (x1 == y1) && (x2 == y2) && (x3 == y3)
26+
&& (x4 == y4) && (x5 == y5) && (x6 == y6) && (x7 == y7)
27+
&& (x8 == y8) && (x9 == y9) && (x10 == y10) && (x11 == y11)
28+
&& (x12 == y12) && (x13 == y13) && (x14 == y14) && (x15 == y15)
29+
}
30+
31+
// (Work around constant-evaluation)
32+
fn id<T>(x: T) -> T { x }
33+
34+
fn main() {
35+
// this signals overflow when checking is on
36+
let x = i8x16(1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
37+
<< id(i8x16(17, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0));
38+
39+
// ... but when checking is off, the fallback will truncate the
40+
// input to its lower three bits (= 1). Note that this is *not*
41+
// the behavior of the x86 processor for 8- and 16-bit types,
42+
// but it is necessary to avoid undefined behavior from LLVM.
43+
//
44+
// We check that here, by ensuring the result has only been
45+
// shifted by one place; if overflow checking is turned off, then
46+
// this assertion will pass (and the compiletest driver will
47+
// report that the test did not produce the error expected above).
48+
assert!(eq_i8x16(x, i8x16(2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)));
49+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
12+
// compile-flags: -C debug-assertions
13+
14+
#![feature(core_simd)]
15+
16+
use std::simd::i32x4;
17+
18+
// (Work around constant-evaluation)
19+
fn id<T>(x: T) -> T { x }
20+
21+
fn main() {
22+
let _x = i32x4(-1, 0, 0, 0) >> id(i32x4(32, 0, 0, 0));
23+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
12+
// compile-flags: -C debug-assertions
13+
14+
#![feature(core_simd)]
15+
16+
use std::simd::i32x4;
17+
18+
// (Work around constant-evaluation)
19+
fn id<T>(x: T) -> T { x }
20+
21+
fn main() {
22+
let _x = i32x4(0, 0, 0, -1) >> id(i32x4(0, 0, 0, -1));
23+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
12+
// compile-flags: -C debug-assertions
13+
14+
#![feature(core_simd)]
15+
16+
use std::simd::i64x2;
17+
18+
// (Work around constant-evaluation)
19+
fn id<T>(x: T) -> T { x }
20+
21+
fn main() {
22+
let _x = i64x2(0, -1) >> id(i64x2(0, 64));
23+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
12+
// compile-flags: -C debug-assertions
13+
14+
// This function is checking that our (type-based) automatic
15+
// truncation does not sidestep the overflow checking.
16+
17+
#![feature(core_simd)]
18+
19+
use std::simd::i8x16;
20+
21+
fn eq_i8x16(i8x16(x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12, x13, x14, x15): i8x16,
22+
i8x16(y0, y1, y2, y3, y4, y5, y6, y7, y8, y9, y10, y11, y12, y13, y14, y15): i8x16)
23+
-> bool
24+
{
25+
(x0 == y0) && (x1 == y1) && (x2 == y2) && (x3 == y3)
26+
&& (x4 == y4) && (x5 == y5) && (x6 == y6) && (x7 == y7)
27+
&& (x8 == y8) && (x9 == y9) && (x10 == y10) && (x11 == y11)
28+
&& (x12 == y12) && (x13 == y13) && (x14 == y14) && (x15 == y15)
29+
}
30+
31+
// (Work around constant-evaluation)
32+
fn id<T>(x: T) -> T { x }
33+
34+
fn main() {
35+
// this signals overflow when checking is on
36+
let x = i8x16(2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
37+
>> id(i8x16(17, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0));
38+
39+
// ... but when checking is off, the fallback will truncate the
40+
// input to its lower three bits (= 1). Note that this is *not*
41+
// the behavior of the x86 processor for 8- and 16-bit types,
42+
// but it is necessary to avoid undefined behavior from LLVM.
43+
//
44+
// We check that here, by ensuring the result is not zero; if
45+
// overflow checking is turned off, then this assertion will pass
46+
// (and the compiletest driver will report that the test did not
47+
// produce the error expected above).
48+
assert!(eq_i8x16(x, i8x16(1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)));
49+
}

src/test/run-pass/issue-24258.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// compile-flags: -C debug-assertions
12+
13+
#![feature(core_simd)]
14+
15+
use std::simd::u32x4;
16+
17+
// (Work around constant-evaluation)
18+
fn id<T>(x: T) -> T { x }
19+
20+
fn eq_u32x4(u32x4(x0, x1, x2, x3): u32x4, u32x4(y0, y1, y2, y3): u32x4) -> bool {
21+
(x0 == y0) && (x1 == y1) && (x2 == y2) && (x3 == y3)
22+
}
23+
24+
fn main() {
25+
assert!(eq_u32x4(u32x4(1, 1, 1, 1) << id(u32x4(1, 1, 1, 1)), u32x4(2, 2, 2, 2)));
26+
}

0 commit comments

Comments
 (0)