Skip to content

Commit 409a5dc

Browse files
x17jirix17jiri
committed
review updates
1 parent 2481435 commit 409a5dc

File tree

13 files changed

+89
-51
lines changed

13 files changed

+89
-51
lines changed

‎compiler/rustc_codegen_gcc/src/builder.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ use rustc_codegen_ssa::traits::{
2727
BaseTypeMethods,
2828
BuilderMethods,
2929
ConstMethods,
30-
ExpectKind,
3130
LayoutTypeMethods,
3231
HasCodegen,
3332
OverflowOp,
@@ -36,6 +35,7 @@ use rustc_codegen_ssa::traits::{
3635
use rustc_data_structures::fx::FxHashSet;
3736
use rustc_middle::bug;
3837
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
38+
use rustc_middle::mir::ExpectKind;
3939
use rustc_middle::ty::{ParamEnv, Ty, TyCtxt};
4040
use rustc_middle::ty::layout::{FnAbiError, FnAbiOfHelpers, FnAbiRequest, HasParamEnv, HasTyCtxt, LayoutError, LayoutOfHelpers, TyAndLayout};
4141
use rustc_span::Span;
@@ -460,15 +460,15 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
460460
cond: RValue<'gcc>,
461461
then_block: Block<'gcc>,
462462
else_block: Block<'gcc>,
463-
_expect: ExpectKind,
463+
_expect: Option<ExpectKind>,
464464
) {
465465
/*
466466
// FIXME: emit expectation
467467
match expect {
468-
ExpectKind::None => {}
469-
ExpectKind::True => self.expect(cond.immediate(), true),
470-
ExpectKind::False => self.expect(cond.immediate(), false),
471-
ExpectKind::Unpredictable => {} // FIXME
468+
Some(ExpectKind::True) => self.expect(cond.immediate(), true),
469+
Some(ExpectKind::False) => self.expect(cond.immediate(), false),
470+
Some(ExpectKind::Unpredictable) => {} // FIXME
471+
None => {}
472472
}
473473
*/
474474

‎compiler/rustc_codegen_llvm/src/builder.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use rustc_codegen_ssa::MemFlags;
1616
use rustc_data_structures::small_c_str::SmallCStr;
1717
use rustc_hir::def_id::DefId;
1818
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
19+
use rustc_middle::mir::ExpectKind;
1920
use rustc_middle::ty::layout::{
2021
FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOfHelpers, TyAndLayout,
2122
};
@@ -202,42 +203,43 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
202203
cond: &'ll Value,
203204
then_llbb: &'ll BasicBlock,
204205
else_llbb: &'ll BasicBlock,
205-
expect: ExpectKind,
206+
expect: Option<ExpectKind>,
206207
) {
207208
// emit the branch instruction
208209
let n = unsafe { llvm::LLVMBuildCondBr(self.llbuilder, cond, then_llbb, else_llbb) };
209210

210211
// emit expectation metadata
211212
match expect {
212-
ExpectKind::None => {}
213-
ExpectKind::True | ExpectKind::False => unsafe {
213+
Some(ExpectKind::True) | Some(ExpectKind::False) => unsafe {
214214
// Use weights 2000 and 1, which is what Clang uses
215215
let s = "branch_weights";
216+
let e = expect.unwrap() == ExpectKind::True;
216217
let v = [
217218
llvm::LLVMMDStringInContext(
218219
self.cx.llcx,
219220
s.as_ptr() as *const c_char,
220221
s.len() as c_uint,
221222
),
222223
// 'then' branch weight
223-
self.cx.const_u32(if expect == ExpectKind::True { 2000 } else { 1 }),
224+
self.cx.const_u32(if e { 2000 } else { 1 }),
224225
// 'else' branch weight
225-
self.cx.const_u32(if expect == ExpectKind::True { 1 } else { 2000 }),
226+
self.cx.const_u32(if e { 1 } else { 2000 }),
226227
];
227228
llvm::LLVMSetMetadata(
228229
n,
229230
llvm::MD_prof as c_uint,
230231
llvm::LLVMMDNodeInContext(self.cx.llcx, v.as_ptr(), v.len() as c_uint),
231232
);
232233
},
233-
ExpectKind::Unpredictable => unsafe {
234+
Some(ExpectKind::Unpredictable) => unsafe {
234235
let v: [&Value; 0] = [];
235236
llvm::LLVMSetMetadata(
236237
n,
237238
llvm::MD_unpredictable as c_uint,
238239
llvm::LLVMMDNodeInContext(self.cx.llcx, v.as_ptr(), v.len() as c_uint),
239240
);
240241
},
242+
None => {}
241243
}
242244
}
243245

‎compiler/rustc_codegen_ssa/src/mir/block.rs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -329,26 +329,25 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
329329
let lltrue = helper.llbb_with_cleanup(self, target);
330330
let llfalse = helper.llbb_with_cleanup(self, targets.otherwise());
331331
if switch_ty == bx.tcx().types.bool {
332-
let expect = if let Some(x) = self.mir[bb].statements.last()
332+
let expect = if let Some(stmt) = self.mir[bb].statements.last()
333333
&& let mir::StatementKind::Intrinsic(box mir::NonDivergingIntrinsic::Expect(
334334
op,
335-
kind,
336-
)) = &x.kind
335+
expect_kind,
336+
)) = &stmt.kind
337337
&& self.codegen_operand(bx, op).immediate() == discr.immediate()
338338
{
339-
match kind {
340-
mir::ExpectKind::True => ExpectKind::True,
341-
mir::ExpectKind::False => ExpectKind::False,
342-
mir::ExpectKind::Unpredictable => ExpectKind::Unpredictable,
343-
}
339+
Some(*expect_kind)
344340
} else {
345-
ExpectKind::None
341+
None
346342
};
347343

348344
// Don't generate trivial icmps when switching on bool.
349345
match test_value {
350346
0 => bx.cond_br_with_expect(discr.immediate(), llfalse, lltrue, expect),
351-
1 => bx.cond_br_with_expect(discr.immediate(), lltrue, llfalse, expect.not()),
347+
1 => {
348+
let expect = expect.and_then(|k| Some(k.not()));
349+
bx.cond_br_with_expect(discr.immediate(), lltrue, llfalse, expect);
350+
}
352351
_ => bug!(),
353352
}
354353
} else {

‎compiler/rustc_codegen_ssa/src/mir/statement.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
7070
let op_val = self.codegen_operand(bx, op);
7171
bx.assume(op_val.immediate());
7272
}
73+
// The Expect intrinsic is processed with the BB terminator
74+
// in codegen_switchint_terminator()
7375
mir::StatementKind::Intrinsic(box NonDivergingIntrinsic::Expect(..)) => {}
7476
mir::StatementKind::Intrinsic(box NonDivergingIntrinsic::CopyNonOverlapping(
7577
mir::CopyNonOverlapping { ref count, ref src, ref dst },

‎compiler/rustc_codegen_ssa/src/traits/builder.rs

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use crate::mir::place::PlaceRef;
1515
use crate::MemFlags;
1616

1717
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
18+
use rustc_middle::mir::ExpectKind;
1819
use rustc_middle::ty::layout::{HasParamEnv, TyAndLayout};
1920
use rustc_middle::ty::Ty;
2021
use rustc_span::Span;
@@ -29,26 +30,6 @@ pub enum OverflowOp {
2930
Mul,
3031
}
3132

32-
#[derive(Copy, Clone, Debug, PartialEq)]
33-
pub enum ExpectKind {
34-
None,
35-
True,
36-
False,
37-
Unpredictable,
38-
}
39-
40-
impl ExpectKind {
41-
#[inline]
42-
pub fn not(&self) -> Self {
43-
match self {
44-
ExpectKind::None => ExpectKind::None,
45-
ExpectKind::True => ExpectKind::False,
46-
ExpectKind::False => ExpectKind::True,
47-
ExpectKind::Unpredictable => ExpectKind::Unpredictable,
48-
}
49-
}
50-
}
51-
5233
pub trait BuilderMethods<'a, 'tcx>:
5334
HasCodegen<'tcx>
5435
+ CoverageInfoBuilderMethods<'tcx>
@@ -95,7 +76,7 @@ pub trait BuilderMethods<'a, 'tcx>:
9576
cond: Self::Value,
9677
then_llbb: Self::BasicBlock,
9778
else_llbb: Self::BasicBlock,
98-
_expect: ExpectKind,
79+
_expect: Option<ExpectKind>,
9980
) {
10081
self.cond_br(cond, then_llbb, else_llbb)
10182
}

‎compiler/rustc_codegen_ssa/src/traits/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub use self::asm::{AsmBuilderMethods, AsmMethods, GlobalAsmOperandRef, InlineAs
3333
pub use self::backend::{
3434
Backend, BackendTypes, CodegenBackend, ExtraBackendMethods, PrintBackendInfo,
3535
};
36-
pub use self::builder::{BuilderMethods, ExpectKind, OverflowOp};
36+
pub use self::builder::{BuilderMethods, OverflowOp};
3737
pub use self::consts::ConstMethods;
3838
pub use self::coverageinfo::CoverageInfoBuilderMethods;
3939
pub use self::debuginfo::{DebugInfoBuilderMethods, DebugInfoMethods};

‎compiler/rustc_middle/src/mir/syntax.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,7 @@ impl StatementKind<'_> {
412412

413413
#[derive(
414414
Clone,
415+
Copy,
415416
TyEncodable,
416417
TyDecodable,
417418
Debug,
@@ -432,6 +433,17 @@ pub enum ExpectKind {
432433
Unpredictable,
433434
}
434435

436+
impl ExpectKind {
437+
#[inline]
438+
pub fn not(&self) -> Self {
439+
match self {
440+
ExpectKind::True => ExpectKind::False,
441+
ExpectKind::False => ExpectKind::True,
442+
ExpectKind::Unpredictable => ExpectKind::Unpredictable,
443+
}
444+
}
445+
}
446+
435447
#[derive(
436448
Clone,
437449
TyEncodable,

‎compiler/rustc_middle/src/mir/visit.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -438,8 +438,10 @@ macro_rules! make_mir_visitor {
438438
}
439439
StatementKind::Intrinsic(box ref $($mutability)? intrinsic) => {
440440
match intrinsic {
441-
NonDivergingIntrinsic::Assume(op) => self.visit_operand(op, location),
442-
NonDivergingIntrinsic::Expect(op, ..) => self.visit_operand(op, location),
441+
NonDivergingIntrinsic::Assume(op)
442+
| NonDivergingIntrinsic::Expect(op, ..)
443+
=> self.visit_operand(op, location),
444+
443445
NonDivergingIntrinsic::CopyNonOverlapping(CopyNonOverlapping { src, dst, count }) => {
444446
self.visit_operand(src, location);
445447
self.visit_operand(dst, location);

‎compiler/rustc_mir_dataflow/src/impls/liveness.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,13 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeTransitiveLiveLocals<'a> {
255255
statement: &mir::Statement<'tcx>,
256256
location: Location,
257257
) {
258+
if let StatementKind::Intrinsic(box mir::NonDivergingIntrinsic::Expect(..)) = statement.kind
259+
{
260+
// don't visit the Expect intrinsic's operand,
261+
// i.e, the expect intrinsic alone will not keep its operand alive
262+
return;
263+
}
264+
258265
// Compute the place that we are storing to, if any
259266
let destination = match &statement.kind {
260267
StatementKind::Assign(assign) => assign.1.is_safe_to_remove().then_some(assign.0),

‎compiler/rustc_mir_transform/src/dead_store_elimination.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,15 @@ pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
8383
match &statement.kind {
8484
StatementKind::Assign(box (place, _))
8585
| StatementKind::SetDiscriminant { place: box place, .. }
86-
| StatementKind::Deinit(box place) => {
86+
| StatementKind::Deinit(box place)
87+
// The Expect intrinsic is different from the other statements in this match arm,
88+
// because it does not write to the 'place'. But if the 'place' is dead,
89+
// the Expect intrinsic can be removed as well.
90+
| StatementKind::Intrinsic(box NonDivergingIntrinsic::Expect(
91+
Operand::Copy(place),
92+
..,
93+
))
94+
=> {
8795
if !place.is_indirect() && !always_live.contains(place.local) {
8896
live.seek_before_primary_effect(loc);
8997
if !live.get().contains(place.local) {

‎compiler/rustc_mir_transform/src/simplify.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,9 @@ impl UsedLocals {
487487
impl<'tcx> Visitor<'tcx> for UsedLocals {
488488
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
489489
match statement.kind {
490+
// Don't count the call to 'Expect' as a use of the argument.
491+
StatementKind::Intrinsic(box NonDivergingIntrinsic::Expect(..)) => {}
492+
490493
StatementKind::Intrinsic(..)
491494
| StatementKind::Retag(..)
492495
| StatementKind::Coverage(..)
@@ -546,6 +549,11 @@ fn remove_unused_definitions_helper(used_locals: &mut UsedLocals, body: &mut Bod
546549
}
547550
StatementKind::Assign(box (place, _)) => used_locals.is_used(place.local),
548551

552+
StatementKind::Intrinsic(box NonDivergingIntrinsic::Expect(
553+
Operand::Copy(place),
554+
..,
555+
)) => used_locals.is_used(place.local),
556+
549557
StatementKind::SetDiscriminant { ref place, .. }
550558
| StatementKind::Deinit(ref place) => used_locals.is_used(place.local),
551559
StatementKind::Nop => false,

‎compiler/rustc_mir_transform/src/simplify_branches.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,25 @@ impl<'tcx> MirPass<'tcx> for SimplifyConstCondition {
3232
continue 'blocks;
3333
}
3434
}
35+
if let StatementKind::Intrinsic(box ref intrinsic) = stmt.kind {
36+
match intrinsic {
37+
NonDivergingIntrinsic::Assume(discr)
38+
| NonDivergingIntrinsic::Expect(discr, ..) => {
39+
if let Operand::Constant(ref c) = discr
40+
&& let Some(constant) = c.const_.try_eval_bool(tcx, param_env)
41+
{
42+
if constant {
43+
stmt.make_nop();
44+
} else {
45+
block.statements.clear();
46+
block.terminator_mut().kind = TerminatorKind::Unreachable;
47+
continue 'blocks;
48+
}
49+
}
50+
}
51+
_ => {}
52+
}
53+
}
3554
}
3655

3756
let terminator = block.terminator_mut();

‎compiler/stable_mir/src/mir/visit.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,8 @@ pub trait MirVisitor {
216216
}
217217
StatementKind::Coverage(coverage) => visit_opaque(coverage),
218218
StatementKind::Intrinsic(intrisic) => match intrisic {
219-
NonDivergingIntrinsic::Assume(operand) => {
220-
self.visit_operand(operand, location);
221-
}
222-
NonDivergingIntrinsic::Expect(operand, ..) => {
219+
NonDivergingIntrinsic::Assume(operand)
220+
| NonDivergingIntrinsic::Expect(operand, ..) => {
223221
self.visit_operand(operand, location);
224222
}
225223
NonDivergingIntrinsic::CopyNonOverlapping(CopyNonOverlapping {

0 commit comments

Comments
 (0)