Skip to content

Commit

Permalink
Fix up merge_from_succ.
Browse files Browse the repository at this point in the history
This function has a variable `changed` that is erroneously used for two
related-but-different purpose:
- to detect if the current element has changed;
- to detect if any elements have changed.

As a result, its use for the first purpose is broken, because if any
prior element changed then the code always thinks the current element
has changed. This is only a performance bug, not a correctness bug,
because we frequently end up calling `assign_unpacked` unnecessarily to
overwrite the element with itself.

This commit adds `any_changed` to correctly distinguish between the two
purposes. This is a small perf win for some benchmarks.
  • Loading branch information
nnethercote committed Feb 3, 2020
1 parent eed12bc commit 0eb297d
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions src/librustc_passes/liveness.rs
Expand Up @@ -822,8 +822,9 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
return false;
}

let mut changed = false;
let mut any_changed = false;
self.indices2(ln, succ_ln, |this, idx, succ_idx| {
let mut changed = false;
let mut rwu = this.rwu_table.get(idx);
let succ_rwu = this.rwu_table.get(succ_idx);
if succ_rwu.reader.is_valid() && !rwu.reader.is_valid() {
Expand All @@ -843,6 +844,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {

if changed {
this.rwu_table.assign_unpacked(idx, rwu);
any_changed = true;
}
});

Expand All @@ -851,9 +853,9 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
ln,
self.ln_str(succ_ln),
first_merge,
changed
any_changed
);
return changed;
return any_changed;
}

// Indicates that a local variable was *defined*; we know that no
Expand Down

0 comments on commit 0eb297d

Please sign in to comment.