From c62f433e46c8f3b122857021acb448e9b8fb7972 Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Sun, 23 Aug 2020 15:00:09 +0200 Subject: [PATCH 1/6] remove_unused_stack_addr_and_stack_load: Remove clone() --- src/optimize/stack2reg.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/optimize/stack2reg.rs b/src/optimize/stack2reg.rs index d6a9d6228..cb68f1cca 100644 --- a/src/optimize/stack2reg.rs +++ b/src/optimize/stack2reg.rs @@ -315,14 +315,13 @@ fn remove_unused_stack_addr_and_stack_load(opt_ctx: &mut OptimizeContext<'_>) { // Replace all unused stack_addr and stack_load instructions with nop. for stack_slot_users in opt_ctx.stack_slot_usage_map.values_mut() { - // FIXME remove clone - for &inst in stack_slot_users.stack_addr.clone().iter() { + while let Some(&inst) = stack_slot_users.stack_addr.iter().next() { if stack_addr_load_insts_users.get(&inst).map(|users| users.is_empty()).unwrap_or(true) { stack_slot_users.remove_unused_stack_addr(&mut opt_ctx.ctx.func, inst); } } - for &inst in stack_slot_users.stack_load.clone().iter() { + while let Some(&inst) = stack_slot_users.stack_load.iter().next() { if stack_addr_load_insts_users.get(&inst).map(|users| users.is_empty()).unwrap_or(true) { stack_slot_users.remove_unused_load(&mut opt_ctx.ctx.func, inst); } From 80ee042a36fd2cbdefc1b1ac09a930976d118a83 Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Sun, 23 Aug 2020 17:31:07 +0200 Subject: [PATCH 2/6] stack2reg: Re-add clone() to stop CI --- src/optimize/stack2reg.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/optimize/stack2reg.rs b/src/optimize/stack2reg.rs index cb68f1cca..444a006f1 100644 --- a/src/optimize/stack2reg.rs +++ b/src/optimize/stack2reg.rs @@ -315,13 +315,14 @@ fn remove_unused_stack_addr_and_stack_load(opt_ctx: &mut OptimizeContext<'_>) { // Replace all unused stack_addr and stack_load instructions with nop. for stack_slot_users in opt_ctx.stack_slot_usage_map.values_mut() { - while let Some(&inst) = stack_slot_users.stack_addr.iter().next() { + // FIXME: Remove clone + for &inst in stack_slot_users.stack_addr.clone().iter() { if stack_addr_load_insts_users.get(&inst).map(|users| users.is_empty()).unwrap_or(true) { stack_slot_users.remove_unused_stack_addr(&mut opt_ctx.ctx.func, inst); } } - while let Some(&inst) = stack_slot_users.stack_load.iter().next() { + for &inst in stack_slot_users.stack_load.clone().iter() { if stack_addr_load_insts_users.get(&inst).map(|users| users.is_empty()).unwrap_or(true) { stack_slot_users.remove_unused_load(&mut opt_ctx.ctx.func, inst); } From b0ea85fb837ab62cd032e18d1429f7b9e2f05726 Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Sun, 23 Aug 2020 17:55:35 +0200 Subject: [PATCH 3/6] stack2reg: Remove self in remove_unused_stack_addr and() remove_unused_load() --- src/optimize/stack2reg.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/optimize/stack2reg.rs b/src/optimize/stack2reg.rs index 444a006f1..2c8fe37a8 100644 --- a/src/optimize/stack2reg.rs +++ b/src/optimize/stack2reg.rs @@ -79,16 +79,14 @@ impl StackSlotUsage { }).collect::>() } - fn remove_unused_stack_addr(&mut self, func: &mut Function, inst: Inst) { + fn remove_unused_stack_addr(func: &mut Function, inst: Inst) { func.dfg.detach_results(inst); func.dfg.replace(inst).nop(); - self.stack_addr.remove(&inst); } - fn remove_unused_load(&mut self, func: &mut Function, load: Inst) { + fn remove_unused_load(func: &mut Function, load: Inst) { func.dfg.detach_results(load); func.dfg.replace(load).nop(); - self.stack_load.remove(&load); } fn remove_dead_store(&mut self, func: &mut Function, store: Inst) { @@ -315,18 +313,21 @@ fn remove_unused_stack_addr_and_stack_load(opt_ctx: &mut OptimizeContext<'_>) { // Replace all unused stack_addr and stack_load instructions with nop. for stack_slot_users in opt_ctx.stack_slot_usage_map.values_mut() { - // FIXME: Remove clone for &inst in stack_slot_users.stack_addr.clone().iter() { if stack_addr_load_insts_users.get(&inst).map(|users| users.is_empty()).unwrap_or(true) { - stack_slot_users.remove_unused_stack_addr(&mut opt_ctx.ctx.func, inst); + StackSlotUsage::remove_unused_stack_addr(&mut opt_ctx.ctx.func, inst); + stack_slot_users.stack_addr.remove(&inst); } } + /* for &inst in stack_slot_users.stack_load.clone().iter() { if stack_addr_load_insts_users.get(&inst).map(|users| users.is_empty()).unwrap_or(true) { - stack_slot_users.remove_unused_load(&mut opt_ctx.ctx.func, inst); + StackSlotUsage::remove_unused_load(&mut opt_ctx.ctx.func, inst); + stack_slot_users.stack_load.remove(&inst); } } + */ } } From eb6df58cdde7c4a83d960ba2e75f5c7421c94689 Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Sun, 23 Aug 2020 18:16:36 +0200 Subject: [PATCH 4/6] stack2reg: Remove clone() using filters --- src/optimize/stack2reg.rs | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/optimize/stack2reg.rs b/src/optimize/stack2reg.rs index 2c8fe37a8..72762f563 100644 --- a/src/optimize/stack2reg.rs +++ b/src/optimize/stack2reg.rs @@ -312,22 +312,19 @@ fn remove_unused_stack_addr_and_stack_load(opt_ctx: &mut OptimizeContext<'_>) { } // Replace all unused stack_addr and stack_load instructions with nop. + let mut func = &mut opt_ctx.ctx.func; for stack_slot_users in opt_ctx.stack_slot_usage_map.values_mut() { - for &inst in stack_slot_users.stack_addr.clone().iter() { - if stack_addr_load_insts_users.get(&inst).map(|users| users.is_empty()).unwrap_or(true) { - StackSlotUsage::remove_unused_stack_addr(&mut opt_ctx.ctx.func, inst); - stack_slot_users.stack_addr.remove(&inst); - } - } - - /* - for &inst in stack_slot_users.stack_load.clone().iter() { - if stack_addr_load_insts_users.get(&inst).map(|users| users.is_empty()).unwrap_or(true) { - StackSlotUsage::remove_unused_load(&mut opt_ctx.ctx.func, inst); - stack_slot_users.stack_load.remove(&inst); - } - } - */ + stack_slot_users + .stack_addr + .iter() + .filter(|inst| stack_addr_load_insts_users.get(inst).map(|users| users.is_empty()).unwrap_or(true)) + .for_each(|inst| StackSlotUsage::remove_unused_stack_addr(&mut func, *inst)); + + stack_slot_users + .stack_load + .iter() + .filter(|inst| stack_addr_load_insts_users.get(inst).map(|users| users.is_empty()).unwrap_or(true)) + .for_each(|inst| StackSlotUsage::remove_unused_load(&mut func, *inst)); } } From 5c8c75b1d2a1fd8146daf71a3ef05acdca406d0b Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Sun, 23 Aug 2020 18:36:42 +0200 Subject: [PATCH 5/6] stack2reg: Drain instead of only iterating --- src/optimize/stack2reg.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/optimize/stack2reg.rs b/src/optimize/stack2reg.rs index 72762f563..d1826a9af 100644 --- a/src/optimize/stack2reg.rs +++ b/src/optimize/stack2reg.rs @@ -316,15 +316,15 @@ fn remove_unused_stack_addr_and_stack_load(opt_ctx: &mut OptimizeContext<'_>) { for stack_slot_users in opt_ctx.stack_slot_usage_map.values_mut() { stack_slot_users .stack_addr - .iter() + .drain() .filter(|inst| stack_addr_load_insts_users.get(inst).map(|users| users.is_empty()).unwrap_or(true)) - .for_each(|inst| StackSlotUsage::remove_unused_stack_addr(&mut func, *inst)); + .for_each(|inst| StackSlotUsage::remove_unused_stack_addr(&mut func, inst)); stack_slot_users .stack_load - .iter() + .drain() .filter(|inst| stack_addr_load_insts_users.get(inst).map(|users| users.is_empty()).unwrap_or(true)) - .for_each(|inst| StackSlotUsage::remove_unused_load(&mut func, *inst)); + .for_each(|inst| StackSlotUsage::remove_unused_load(&mut func, inst)); } } From cb386896ee4df8f76758737044abe9300d6f8802 Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Sun, 23 Aug 2020 21:44:00 +0200 Subject: [PATCH 6/6] stack2reg: Switch to hashbrown::HashSet --- Cargo.lock | 8 ++++++++ Cargo.toml | 1 + src/optimize/stack2reg.rs | 21 +++++++++++++-------- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 77d1fd0b4..b072d9bd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,5 +1,11 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +[[package]] +name = "ahash" +version = "0.3.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8fd72866655d1904d6b0997d0b07ba561047d070fbe29de039031c641b61217" + [[package]] name = "anyhow" version = "1.0.32" @@ -197,6 +203,7 @@ version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34f595585f103464d8d2f6e9864682d74c1601fed5e07d62b1c9058dba8246fb" dependencies = [ + "ahash", "autocfg", ] @@ -325,6 +332,7 @@ dependencies = [ "cranelift-object", "cranelift-simplejit", "gimli", + "hashbrown", "indexmap", "libloading", "object", diff --git a/Cargo.toml b/Cargo.toml index e178e2992..858a0f0e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ byteorder = "1.2.7" indexmap = "1.0.2" cfg-if = "0.1.10" libloading = { version = "0.6.0", optional = true } +hashbrown = "0.8.1" # Uncomment to use local checkout of cranelift #[patch."https://github.com/bytecodealliance/wasmtime/"] diff --git a/src/optimize/stack2reg.rs b/src/optimize/stack2reg.rs index d1826a9af..aceced41f 100644 --- a/src/optimize/stack2reg.rs +++ b/src/optimize/stack2reg.rs @@ -13,12 +13,15 @@ use std::collections::BTreeMap; use std::fmt; use std::ops::Not; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashSet, FxHasher}; use cranelift_codegen::cursor::{Cursor, FuncCursor}; use cranelift_codegen::ir::{InstructionData, Opcode, ValueDef}; use cranelift_codegen::ir::immediates::Offset32; +use hashbrown::HashSet; +use std::hash::BuildHasherDefault; + use crate::prelude::*; /// Workaround for `StackSlot` not implementing `Ord`. @@ -45,9 +48,9 @@ impl Ord for OrdStackSlot { #[derive(Debug, Default)] struct StackSlotUsage { - stack_addr: FxHashSet, - stack_load: FxHashSet, - stack_store: FxHashSet, + stack_addr: HashSet>, + stack_load: HashSet>, + stack_store: HashSet>, } impl StackSlotUsage { @@ -313,17 +316,19 @@ fn remove_unused_stack_addr_and_stack_load(opt_ctx: &mut OptimizeContext<'_>) { // Replace all unused stack_addr and stack_load instructions with nop. let mut func = &mut opt_ctx.ctx.func; + + // drain_filter() on hashbrown::HashSet drains the items that do *not* match the + // predicate. Once hashbrown gets updated to match the behaviour of std::drain_filter + // (0.8.2), the predicate will have to be reversed for stack_slot_users in opt_ctx.stack_slot_usage_map.values_mut() { stack_slot_users .stack_addr - .drain() - .filter(|inst| stack_addr_load_insts_users.get(inst).map(|users| users.is_empty()).unwrap_or(true)) + .drain_filter(|inst| !(stack_addr_load_insts_users.get(inst).map(|users| users.is_empty()).unwrap_or(true))) .for_each(|inst| StackSlotUsage::remove_unused_stack_addr(&mut func, inst)); stack_slot_users .stack_load - .drain() - .filter(|inst| stack_addr_load_insts_users.get(inst).map(|users| users.is_empty()).unwrap_or(true)) + .drain_filter(|inst| !(stack_addr_load_insts_users.get(inst).map(|users| users.is_empty()).unwrap_or(true))) .for_each(|inst| StackSlotUsage::remove_unused_load(&mut func, inst)); } }