Skip to content

Commit

Permalink
Don't perform swap when src == dst. #5041
Browse files Browse the repository at this point in the history
  • Loading branch information
brson committed Feb 21, 2013
1 parent 831840a commit 423843e
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 4 deletions.
26 changes: 22 additions & 4 deletions src/librustc/middle/trans/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,11 +491,29 @@ fn trans_rvalue_stmt_unadjusted(bcx: block, expr: @ast::expr) -> block {
ast::expr_swap(dst, src) => {
let dst_datum = unpack_datum!(bcx, trans_lvalue(bcx, dst));
let src_datum = unpack_datum!(bcx, trans_lvalue(bcx, src));
let scratch = scratch_datum(bcx, dst_datum.ty, false);

let bcx = dst_datum.move_to_datum(bcx, INIT, scratch);
let bcx = src_datum.move_to_datum(bcx, INIT, dst_datum);
return scratch.move_to_datum(bcx, INIT, src_datum);
// If the source and destination are the same, then don't swap.
// Avoids performing an overlapping memcpy
let dst_datum_ref = dst_datum.to_ref_llval(bcx);
let src_datum_ref = src_datum.to_ref_llval(bcx);
let cmp = ICmp(bcx, lib::llvm::IntEQ,
src_datum_ref,
dst_datum_ref);

let swap_cx = base::sub_block(bcx, ~"swap");
let next_cx = base::sub_block(bcx, ~"next");

CondBr(bcx, cmp, next_cx.llbb, swap_cx.llbb);

let scratch = scratch_datum(swap_cx, dst_datum.ty, false);

let swap_cx = dst_datum.move_to_datum(swap_cx, INIT, scratch);
let swap_cx = src_datum.move_to_datum(swap_cx, INIT, dst_datum);
let swap_cx = scratch.move_to_datum(swap_cx, INIT, src_datum);

Br(swap_cx, next_cx.llbb);

return next_cx;
}
ast::expr_assign_op(op, dst, src) => {
return trans_assign_op(bcx, expr, op, dst, src);
Expand Down
45 changes: 45 additions & 0 deletions src/test/run-pass/swap-overlapping.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2013 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.

// Issue #5041 - avoid overlapping memcpy when src and dest of a swap are the same

pub fn main() {
let mut test = TestDescAndFn {
desc: TestDesc {
name: DynTestName(~"test"),
should_fail: false
},
testfn: DynTestFn(|| ()),
};
do_swap(&mut test);
}

fn do_swap(test: &mut TestDescAndFn) {
*test <-> *test;
}

pub enum TestName {
DynTestName(~str)
}

pub enum TestFn {
DynTestFn(~fn()),
DynBenchFn(~fn(&mut int))
}

pub struct TestDesc {
name: TestName,
should_fail: bool
}

pub struct TestDescAndFn {
desc: TestDesc,
testfn: TestFn,
}

5 comments on commit 423843e

@bors
Copy link
Contributor

@bors bors commented on 423843e Feb 21, 2013

Choose a reason for hiding this comment

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

saw approval from brson
at brson@423843e

@bors
Copy link
Contributor

@bors bors commented on 423843e Feb 21, 2013

Choose a reason for hiding this comment

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

merging brson/rust/swap = 423843e into auto

@bors
Copy link
Contributor

@bors bors commented on 423843e Feb 21, 2013

Choose a reason for hiding this comment

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

brson/rust/swap = 423843e merged ok, testing candidate = 8f8f0ec

@bors
Copy link
Contributor

@bors bors commented on 423843e Feb 21, 2013

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 423843e Feb 21, 2013

Choose a reason for hiding this comment

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

fast-forwarding incoming to auto = 8f8f0ec

Please sign in to comment.