Skip to content

Commit

Permalink
Back out "update inliner to editable cfg"
Browse files Browse the repository at this point in the history
Summary: Introduced some nondeterminism

Reviewed By: beicy

Differential Revision: D50562050

fbshipit-source-id: f1aa3d7cb0be4ba11026461923e7f5409c2fb104
  • Loading branch information
ssj933 authored and facebook-github-bot committed Oct 23, 2023
1 parent 58b38fb commit 6b92713
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 19 deletions.
1 change: 1 addition & 0 deletions opt/methodinline/BridgeSynthInlinePass.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class BridgeSynthInlinePass : public Pass {
{SpuriousGetClassCallsInterned, RequiresAndPreserves},
};
}
bool is_cfg_legacy() override { return true; }

void run_pass(DexStoresVector&, ConfigFiles&, PassManager&) override;
};
2 changes: 2 additions & 0 deletions opt/methodinline/IntraDexInlinePass.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,7 @@ class IntraDexInlinePass : public Pass {
};
}

bool is_cfg_legacy() override { return true; }

void run_pass(DexStoresVector&, ConfigFiles&, PassManager&) override;
};
2 changes: 2 additions & 0 deletions opt/methodinline/LocalMethodInlinePass.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,7 @@ class LocalMethodInlinePass : public Pass {
};
}

bool is_cfg_legacy() override { return true; }

void run_pass(DexStoresVector&, ConfigFiles&, PassManager&) override;
};
2 changes: 2 additions & 0 deletions opt/methodinline/MethodInlinePass.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,7 @@ class MethodInlinePass : public Pass {
};
}

bool is_cfg_legacy() override { return true; }

void run_pass(DexStoresVector&, ConfigFiles&, PassManager&) override;
};
2 changes: 2 additions & 0 deletions opt/methodinline/PerfMethodInlinePass.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class PerfMethodInlinePass : public Pass {

void bind_config() override;

bool is_cfg_legacy() override { return true; }

void run_pass(DexStoresVector&, ConfigFiles&, PassManager&) override;

private:
Expand Down
1 change: 1 addition & 0 deletions opt/regalloc-fast/FastRegAlloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class FastRegAllocPass : public Pass {
void eval_pass(DexStoresVector& stores,
ConfigFiles& conf,
PassManager& mgr) override;
bool is_cfg_legacy() override { return true; }
void run_pass(DexStoresVector&, ConfigFiles&, PassManager&) override;

private:
Expand Down
1 change: 1 addition & 0 deletions opt/shrinker/ShrinkerPass.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class ShrinkerPass : public Pass {
}

void bind_config() override;
bool is_cfg_legacy() override { return true; }
void run_pass(DexStoresVector&, ConfigFiles&, PassManager&) override;

private:
Expand Down
27 changes: 11 additions & 16 deletions service/method-inliner/MethodInliner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,22 +334,10 @@ SameImplementationMap get_same_implementation_map(
SameImplementation same_implementation{nullptr, {}};
auto consider_method = [&](DexMethod* method) {
always_assert(method->get_code());
// TODO "structural_equals" feature of editable cfg hasn't been
// implenmented yet. Currently, we still need to use
// irlist::structural_equals. Therefore, we need to clear_cfg before
// finding equivalent methods. Once structural_equals of editable cfg is
// added, the following clear_cfg will be removed.
if (same_implementation.representative != nullptr) {
method->get_code()->clear_cfg();
same_implementation.representative->get_code()->clear_cfg();
if (!method->get_code()->structural_equals(
*same_implementation.representative->get_code())) {
method->get_code()->build_cfg();
same_implementation.representative->get_code()->build_cfg();
return false;
}
method->get_code()->build_cfg();
same_implementation.representative->get_code()->build_cfg();
if (same_implementation.representative != nullptr &&
!method->get_code()->structural_equals(
*same_implementation.representative->get_code())) {
return false;
}
if (same_implementation.representative == nullptr ||
compare_dexmethods(method, same_implementation.representative)) {
Expand Down Expand Up @@ -813,6 +801,10 @@ void run_inliner(DexStoresVector& stores,
get_same_implementation_map(scope, *method_override_graph));
}

walk::parallel::code(scope, [](DexMethod*, IRCode& code) {
code.build_cfg(/* editable */ true);
});

if (inliner_config.virtual_inline && inliner_config.true_virtual_inline) {
gather_true_virtual_methods(*method_override_graph, *non_virtual, scope,
*same_implementation_map,
Expand Down Expand Up @@ -849,6 +841,9 @@ void run_inliner(DexStoresVector& stores,
/* configured_finalish_field_names */ {}, local_only);
inliner.inline_methods(/* need_deconstruct */ false);

walk::parallel::code(scope,
[](DexMethod*, IRCode& code) { code.clear_cfg(); });

// delete all methods that can be deleted
auto inlined = inliner.get_inlined();
size_t inlined_count = inlined.size();
Expand Down
43 changes: 40 additions & 3 deletions service/shrinker/Shrinker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ Shrinker::ShrinkerForest load(const std::string& filename) {
}

bool should_shrink(IRCode* code, const Shrinker::ShrinkerForest& forest) {
if (!code->editable_cfg_built()) {
code->build_cfg(/* editable= */ true);
}
const auto& cfg = code->cfg();

size_t regs = cfg.get_registers_size();
Expand Down Expand Up @@ -206,10 +209,12 @@ void Shrinker::shrink_code(
DexType* declaring_type,
DexProto* proto,
const std::function<std::string()>& method_describer) {
always_assert(code->editable_cfg_built());
bool editable_cfg_built = code->editable_cfg_built();
// force simplification/linearization of any existing editable cfg once, and
// forget existing cfg for a clean start
code->cfg().recompute_registers_size();
if (editable_cfg_built) {
code->cfg().recompute_registers_size();
}
code->clear_cfg();

constant_propagation::Transform::Stats const_prop_stats;
Expand All @@ -218,9 +223,12 @@ void Shrinker::shrink_code(
LocalDce::Stats local_dce_stats;
dedup_blocks_impl::Stats dedup_blocks_stats;

code->build_cfg();
if (m_config.run_const_prop) {
auto timer = m_const_prop_timer.scope();
if (!code->editable_cfg_built()) {
code->build_cfg(/* editable */ true);
}

constant_propagation::Transform::Config config;
config.pure_methods = &m_pure_methods;
const_prop_stats = constant_propagation(is_static, declaring_type, proto,
Expand All @@ -229,6 +237,10 @@ void Shrinker::shrink_code(

if (m_config.run_cse) {
auto timer = m_cse_timer.scope();
if (!code->editable_cfg_built()) {
code->build_cfg(/* editable */ true);
}

cse_impl::CommonSubexpressionElimination cse(
m_cse_shared_state.get(), code->cfg(), is_static, is_init_or_clinit,
declaring_type, proto->get_args());
Expand All @@ -238,13 +250,21 @@ void Shrinker::shrink_code(

if (m_config.run_copy_prop) {
auto timer = m_copy_prop_timer.scope();
if (!code->editable_cfg_built()) {
code->build_cfg(/* editable */ true);
}

copy_prop_stats =
copy_propagation(code, is_static, declaring_type, proto->get_rtype(),
proto->get_args(), method_describer);
}

if (m_config.run_local_dce) {
auto timer = m_local_dce_timer.scope();
if (!code->editable_cfg_built()) {
code->build_cfg(/* editable */ true);
}

local_dce_stats =
local_dce(code, /* normalize_new_instances */ true, declaring_type);
}
Expand All @@ -254,6 +274,9 @@ void Shrinker::shrink_code(
if (!traceEnabled(MMINL, mminl_level)) {
return stats_t{};
}
if (!code->editable_cfg_built()) {
code->build_cfg(/* editable= */ true);
}
const auto& cfg = code->cfg();

size_t regs_before = cfg.get_registers_size();
Expand Down Expand Up @@ -297,13 +320,21 @@ void Shrinker::shrink_code(

if (m_config.run_fast_reg_alloc) {
auto timer = m_reg_alloc_timer.scope();
if (!code->editable_cfg_built()) {
code->build_cfg(/* editable= */ true);
}

auto allocator =
fastregalloc::LinearScanAllocator(code, is_static, method_describer);
allocator.allocate();
}

if (m_config.run_dedup_blocks) {
auto timer = m_dedup_blocks_timer.scope();
if (!code->editable_cfg_built()) {
code->build_cfg(/* editable */ true);
}

dedup_blocks_impl::Config config;
dedup_blocks_impl::DedupBlocks dedup_blocks(
&config, code, is_static, declaring_type, proto->get_args());
Expand All @@ -322,6 +353,12 @@ void Shrinker::shrink_code(
std::get<2>(data_after_dedup), std::get<3>(data_after_dedup));
}

if (editable_cfg_built && !code->editable_cfg_built()) {
code->build_cfg(/* editable */ true);
} else if (!editable_cfg_built && code->editable_cfg_built()) {
code->clear_cfg();
}

std::lock_guard<std::mutex> guard(m_stats_mutex);
m_const_prop_stats += const_prop_stats;
m_cse_stats += cse_stats;
Expand Down

0 comments on commit 6b92713

Please sign in to comment.