From c03eba2d0833a21205b00e4b71e920df709a2658 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 28 Aug 2020 14:20:14 -0700 Subject: [PATCH] Add `FIXME` for faster cached block transfer functions I've tried a few ways of implementing this, but each fell short. Adding an auxiliary `_Idx` associated type to `Analysis` that defaults to `!` but is overridden in the blanket impl of `Analysis` for `A: GenKillAnalysis` to `A::Idx` seems promising, but the trait solver is unable to prove equivalence between `A::Idx` and `A::_Idx` within the overridden version of `into_engine`. Without full-featured specialization, removing `into_engine` or splitting it into a different trait would have a significant ergonomic penalty. Alternatively, we could erase the index type and store a `GenKillSet` as well as a function pointer for transmuting between `&mut A::Domain` and `&mut BitSet` in the hopes that LLVM can devirtualize a simple function pointer better than the boxed closure. However, this is brittle, requires `unsafe` code, and doesn't work for index types that aren't the same size as a `u32` (e.g. `usize`) since `GenKillSet` stores a `HybridBitSet`, which may be a `Vec`. Perhaps safe transmute could help here? --- compiler/rustc_mir/src/dataflow/framework/engine.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/rustc_mir/src/dataflow/framework/engine.rs b/compiler/rustc_mir/src/dataflow/framework/engine.rs index 07f5ab5e7bd..d3ad42f6bbc 100644 --- a/compiler/rustc_mir/src/dataflow/framework/engine.rs +++ b/compiler/rustc_mir/src/dataflow/framework/engine.rs @@ -87,6 +87,11 @@ where analysis: A, /// Cached, cumulative transfer functions for each block. + // + // FIXME(ecstaticmorse): This boxed `Fn` trait object is invoked inside a tight loop for + // gen/kill problems on cyclic CFGs. This is not ideal, but it doesn't seem to degrade + // performance in practice. I've tried a few ways to avoid this, but they have downsides. See + // the message for the commit that added this FIXME for more information. apply_trans_for_block: Option>, }