From 0eb297d3e04e618b96e668e8e9ed9ddf66d671fb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 3 Feb 2020 12:11:11 +1100 Subject: [PATCH 1/2] 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 7718139f6e924..606947bfbdd7e 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 From d62b6f204733d255a3e943388ba99f14b053bf4a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 3 Feb 2020 13:32:59 +1100 Subject: [PATCH 2/2] Pull out a special case in `merge_from_succ`. This is a small perf win. --- src/librustc_passes/liveness.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index 606947bfbdd7e..b355a47c39470 100644 --- a/src/librustc_passes/liveness.rs +++ b/src/librustc_passes/liveness.rs @@ -824,6 +824,12 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { let mut any_changed = false; self.indices2(ln, succ_ln, |this, idx, succ_idx| { + // This is a special case, pulled out from the code below, where we + // don't have to do anything. It occurs about 60-70% of the time. + if this.rwu_table.packed_rwus[succ_idx] == INV_INV_FALSE { + return; + } + let mut changed = false; let mut rwu = this.rwu_table.get(idx); let succ_rwu = this.rwu_table.get(succ_idx);