Handle block expressions a little nicer.

This handles block expressions based on the kind of their trailing
expression (if there is one), rather than always using DPS. This is to
allow `unsafe { likely(foo) }` to work as expected, but also reduces the
number of temporary allocas in some cases.

Also adds a codegen test to make sure the intrinsics are being
codegenned in a way that means they are having an effect.

This commit also fixes a small bug I noticed in the translation of
overflow checking that meant the branch hints weren't actually taking
effect.

Author: Aatch

src/librustc_trans/trans/build.rs

@@ -31,7 +31,14 @@ pub fn terminate(cx: Block, _: &str) {
 
 pub fn check_not_terminated(cx: Block) {
     if cx.terminated.get() {
-        panic!("already terminated!");
+        let fcx = cx.fcx;
+        if let Some(span) = fcx.span {
+            cx.tcx().sess.span_bug(
+                span,
+                "already terminated!");
+        } else {
+            cx.tcx().sess.bug("already terminated!");
+        }
     }
 }
 

src/librustc_trans/trans/controlflow.rs

@@ -19,6 +19,7 @@ use trans::cleanup::CleanupMethods;
 use trans::cleanup;
 use trans::common::*;
 use trans::consts;
+use trans::datum;
 use trans::debuginfo;
 use trans::debuginfo::{DebugLoc, ToDebugLoc};
 use trans::expr;
@@ -144,6 +145,48 @@ pub fn trans_block<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
     return bcx;
 }
 
+/// Same as `trans_block` except it returns a `DatumBlock` instead of storing the
+/// the result to a destination. This avoids going through a temporary when it's
+/// not needed, primarily to ensure that `unsafe { likely(cond) }` and similar patterns
+/// work.
+pub fn trans_block_datum<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
+                                     b: &ast::Block)
+                                     -> datum::DatumBlock<'blk, 'tcx, datum::Expr> {
+    let _icx = push_ctxt("trans_block_datum");
+
+    if bcx.unreachable.get() {
+        let llval = C_nil(bcx.ccx());
+        let datum = datum::immediate_rvalue(llval, ty::mk_nil(bcx.tcx()));
+        return datum::DatumBlock {bcx: bcx, datum: datum.to_expr_datum()};
+    }
+
+    let fcx = bcx.fcx;
+    let mut bcx = bcx;
+
+    let cleanup_debug_loc =
+        debuginfo::get_cleanup_debug_loc_for_ast_node(bcx.ccx(), b.id, b.span, true);
+    fcx.push_ast_cleanup_scope(cleanup_debug_loc);
+
+    for s in &b.stmts {
+        bcx = trans_stmt(bcx, &**s);
+    }
+
+    let datum = match b.expr {
+        Some(ref e) if !bcx.unreachable.get() => {
+            unpack_datum!(bcx, expr::trans(bcx, &**e))
+        }
+        _ => {
+            let llval = C_nil(bcx.ccx());
+            datum::immediate_rvalue(llval, ty::mk_nil(bcx.tcx())).to_expr_datum()
+        }
+    };
+
+    bcx = fcx.pop_and_trans_ast_cleanup_scope(bcx, b.id);
+
+    datum::DatumBlock {bcx: bcx, datum: datum}
+}
+
+
 pub fn trans_if<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                             if_id: ast::NodeId,
                             cond: &ast::Expr,

src/librustc_trans/trans/expr.rs

@@ -579,6 +579,8 @@ fn trans_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
 
     debuginfo::set_source_location(bcx.fcx, expr.id, expr.span);
 
+    let ty = expr_ty(bcx, expr);
+
     return match expr_kind(bcx, expr) {
         ty::LvalueExpr | ty::RvalueDatumExpr => {
             let datum = unpack_datum!(bcx, {
@@ -590,11 +592,10 @@ fn trans_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
 
         ty::RvalueStmtExpr => {
             bcx = trans_rvalue_stmt_unadjusted(bcx, expr);
-            nil(bcx, expr_ty(bcx, expr))
+            nil(bcx, ty)
         }
 
         ty::RvalueDpsExpr => {
-            let ty = expr_ty(bcx, expr);
             if type_is_zero_size(bcx.ccx(), ty) {
                 bcx = trans_rvalue_dps_unadjusted(bcx, expr, Ignore);
                 nil(bcx, ty)
@@ -631,6 +632,7 @@ fn trans_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
 // Get the appropriate expression kind for the expression. Most of the time this just uses
 // ty::expr_kind, but `ExprCall`s can be treated as `RvalueDatumExpr`s in some cases.
 fn expr_kind<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, expr: &ast::Expr) -> ty::ExprKind {
+    let ty = expr_ty(bcx, expr);
     match expr.node {
         ast::ExprCall(ref f, _) if !bcx.tcx().is_method_call(expr.id) => {
             if let ast::ExprPath(..) = f.node {
@@ -655,6 +657,21 @@ fn expr_kind<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, expr: &ast::Expr) -> ty::ExprKi
                 }
             }
         }
+        ast::ExprBlock(ref b) if !bcx.fcx.type_needs_drop(ty) => {
+            // Only consider the final expression if it's reachable
+            let reachable = if let Some(ref cfg) = bcx.fcx.cfg {
+                cfg.node_is_reachable(expr.id)
+            } else {
+                true
+            };
+            // Use the kind of the last expression in the block, since
+            // it's the only one that actually matters
+            if let Some(ref expr) = b.expr {
+                if reachable {
+                    return expr_kind(bcx, expr);
+                }
+            }
+        }
         _ => ()
     }
 
@@ -757,6 +774,9 @@ fn trans_datum_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                 DatumBlock::new(bcx, datum.to_expr_datum())
             }
         }
+        ast::ExprBlock(ref b) => {
+            controlflow::trans_block_datum(bcx, b)
+        }
         _ => {
             bcx.tcx().sess.span_bug(
                 expr.span,
@@ -1079,6 +1099,9 @@ fn trans_rvalue_stmt_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
         ast::ExprInlineAsm(ref a) => {
             asm::trans_inline_asm(bcx, a)
         }
+        ast::ExprBlock(ref b) => {
+            controlflow::trans_block(bcx, b, Ignore)
+        }
         _ => {
             bcx.tcx().sess.span_bug(
                 expr.span,
@@ -1097,6 +1120,10 @@ fn trans_rvalue_dps_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
     let mut bcx = bcx;
     let tcx = bcx.tcx();
 
+    if bcx.unreachable.get() {
+        return bcx;
+    }
+
     debuginfo::set_source_location(bcx.fcx, expr.id, expr.span);
 
     match expr.node {
@@ -2479,13 +2506,10 @@ impl OverflowOpViaIntrinsic {
 
         let val = Call(bcx, llfn, &[lhs, rhs], None, binop_debug_loc);
         let result = ExtractValue(bcx, val, 0); // iN operation result
-        let overflow = ExtractValue(bcx, val, 1); // i1 "did it overflow?"
-
-        let cond = ICmp(bcx, llvm::IntEQ, overflow, C_integral(Type::i1(bcx.ccx()), 1, false),
-                        binop_debug_loc);
+        let cond = ExtractValue(bcx, val, 1); // i1 "did it overflow?"
 
         let expect = bcx.ccx().get_intrinsic(&"llvm.expect.i1");
-        Call(bcx, expect, &[cond, C_integral(Type::i1(bcx.ccx()), 0, false)],
+        let cond = Call(bcx, expect, &[cond, C_integral(Type::i1(bcx.ccx()), 0, false)],
              None, binop_debug_loc);
 
         let bcx =

src/test/codegen/expect.rs

@@ -0,0 +1,56 @@
+// Copyright 2015 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -C no-prepopulate-passes
+
+// The expect intrinsic needs to have the output from the call fed directly
+// into the branch, these tests make sure that happens
+
+#![feature(core)]
+
+use std::intrinsics::{likely,unlikely};
+
+// CHECK-LABEL: direct_likely
+#[no_mangle]
+pub fn direct_likely(x: i32) {
+    unsafe {
+        // CHECK: [[VAR:%[0-9]+]] = call i1 @llvm.expect.i1(i1 %{{.*}}, i1 true)
+        // CHECK: br i1 [[VAR]], label %{{.+}}, label %{{.*}}
+        if likely(x == 1) {}
+    }
+}
+
+// CHECK-LABEL: direct_unlikely
+#[no_mangle]
+pub fn direct_unlikely(x: i32) {
+    unsafe {
+        // CHECK: [[VAR:%[0-9]+]] = call i1 @llvm.expect.i1(i1 %{{.*}}, i1 false)
+        // CHECK: br i1 [[VAR]], label %{{.+}}, label %{{.*}}
+        if unlikely(x == 1) {}
+    }
+}
+
+// CHECK-LABEL: wrapped_likely
+// Make sure you can wrap just the call to the intrinsic in `unsafe` and still
+// have it work
+#[no_mangle]
+pub fn wrapped_likely(x: i32) {
+    // CHECK: [[VAR:%[0-9]+]] = call i1 @llvm.expect.i1(i1 %{{.*}}, i1 true)
+    // CHECK: br i1 [[VAR]], label %{{.+}}, label %{{.*}}
+    if unsafe { likely(x == 1) } {}
+}
+
+// CHECK-LABEL: wrapped_unlikely
+#[no_mangle]
+pub fn wrapped_unlikely(x: i32) {
+    // CHECK: [[VAR:%[0-9]+]] = call i1 @llvm.expect.i1(i1 %{{.*}}, i1 false)
+    // CHECK: br i1 [[VAR]], label %{{.+}}, label %{{.*}}
+    if unsafe { unlikely(x == 1) } {}
+}