From 0eb297d3e04e618b96e668e8e9ed9ddf66d671fb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 3 Feb 2020 12:11:11 +1100 Subject: [PATCH] Fix up `merge_from_succ`. 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. --- src/librustc_passes/liveness.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index 7718139f6e9..606947bfbdd 100644 --- a/src/librustc_passes/liveness.rs +++ b/src/librustc_passes/liveness.rs @@ -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() { @@ -843,6 +844,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { if changed { this.rwu_table.assign_unpacked(idx, rwu); + any_changed = true; } }); @@ -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