From 2092f2b9758c4cd8a6a7a5c776048da036bfab72 Mon Sep 17 00:00:00 2001 From: Martin Brain Date: Thu, 28 Sep 2023 15:53:35 +0100 Subject: [PATCH] Refactor three-way-merge so that it makes more sense Given that the operation is taking the diff between function start and function end and applying it to a base, you would think that it should only have three domains. It previously had four and then copied the entire calling domain over the "this" domain, which was not otherwise used. This patch simplifies this and removes that oddity. --- .../three_way_merge_abstract_interpreter.cpp | 21 +++++++++---------- .../variable_sensitivity_dependence_graph.cpp | 18 +++++++--------- .../variable_sensitivity_dependence_graph.h | 1 - .../variable_sensitivity_domain.cpp | 5 ----- .../variable_sensitivity_domain.h | 13 ++++++------ 5 files changed, 24 insertions(+), 34 deletions(-) diff --git a/src/analyses/variable-sensitivity/three_way_merge_abstract_interpreter.cpp b/src/analyses/variable-sensitivity/three_way_merge_abstract_interpreter.cpp index 8735c499b913..86b588e4c28c 100644 --- a/src/analyses/variable-sensitivity/three_way_merge_abstract_interpreter.cpp +++ b/src/analyses/variable-sensitivity/three_way_merge_abstract_interpreter.cpp @@ -160,22 +160,21 @@ bool ai_three_way_merget::visit_edge_function_call( *this, ns); - // TODO : this is probably needed to avoid three_way_merge modifying one of - // its arguments as it goes. A better solution would be to refactor - // merge_three_way_function_return. - const std::unique_ptr ptr_s_working_copy( - make_temporary_state(s_working)); + // The base for the three way merge is the call site + const std::unique_ptr ptr_call_site_working( + make_temporary_state(get_state(p_call_site))); + auto tmp2 = + dynamic_cast(&(*ptr_call_site_working)); + INVARIANT(tmp2 != nullptr, "Three-way merge requires domain support"); + variable_sensitivity_domaint &s_call_site_working = *tmp2; log.progress() << "three way merge... "; - s_working.merge_three_way_function_return( - get_state(p_call_site), - get_state(p_callee_start), - *ptr_s_working_copy, - ns); + s_call_site_working.merge_three_way_function_return( + get_state(p_callee_start), s_working, ns); log.progress() << "merging... "; if( - merge(s_working, p_callee_end, p_return_site) || + merge(s_call_site_working, p_callee_end, p_return_site) || (return_step.first == ai_history_baset::step_statust::NEW && !s_working.is_bottom())) { diff --git a/src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.cpp b/src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.cpp index 00ebc3328630..12f5294fda4b 100644 --- a/src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.cpp +++ b/src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.cpp @@ -438,20 +438,18 @@ bool variable_sensitivity_dependence_domaint::merge( } /** - * Perform a context aware merge of the changes that have been applied - * between function_start and the current state. Anything that has not been - * modified will be taken from the \p function_call domain. - * - * \param function_call: The local of the merge - values from here will be - * taken if they have not been modified - * \param function_start: The base of the merge - changes that have been made + * Merges just the things that have changes between + * "function_start" and "function_end" into "this". + * To be used correctly "this" must be derived from the function + * call site. Anything that is not modified in the function (such + * as globals) will not be changed. + * \param function_start: The base of the diff - changes that have been made * between here and the end will be retained. * \param function_end: The end of the merge - changes that have been made -/// between the start and here will be retained. + * between the start and here will be retained. * \param ns: The global namespace */ void variable_sensitivity_dependence_domaint::merge_three_way_function_return( - const ai_domain_baset &function_call, const ai_domain_baset &function_start, const ai_domain_baset &function_end, const namespacet &ns) @@ -462,7 +460,7 @@ void variable_sensitivity_dependence_domaint::merge_three_way_function_return( // the three way merge is that the underlying variable sensitivity domain // does its three way merge. variable_sensitivity_domaint::merge_three_way_function_return( - function_call, function_start, function_end, ns); + function_start, function_end, ns); } /** diff --git a/src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.h b/src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.h index 5f98578d6cf0..c3a8cad69f33 100644 --- a/src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.h +++ b/src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.h @@ -132,7 +132,6 @@ class variable_sensitivity_dependence_domaint trace_ptrt to) override; void merge_three_way_function_return( - const ai_domain_baset &function_call, const ai_domain_baset &function_start, const ai_domain_baset &function_end, const namespacet &ns) override; diff --git a/src/analyses/variable-sensitivity/variable_sensitivity_domain.cpp b/src/analyses/variable-sensitivity/variable_sensitivity_domain.cpp index a0f9f5249fac..b348096522c8 100644 --- a/src/analyses/variable-sensitivity/variable_sensitivity_domain.cpp +++ b/src/analyses/variable-sensitivity/variable_sensitivity_domain.cpp @@ -417,14 +417,10 @@ bool variable_sensitivity_domaint::ignore_function_call_transform( } void variable_sensitivity_domaint::merge_three_way_function_return( - const ai_domain_baset &function_call, const ai_domain_baset &function_start, const ai_domain_baset &function_end, const namespacet &ns) { - const variable_sensitivity_domaint &cast_function_call = - static_cast(function_call); - const variable_sensitivity_domaint &cast_function_start = static_cast(function_start); @@ -443,7 +439,6 @@ void variable_sensitivity_domaint::merge_three_way_function_return( std::back_inserter(modified_symbols), [&ns](const irep_idt &id) { return ns.lookup(id).symbol_expr(); }); - abstract_state = cast_function_call.abstract_state; for(const auto &symbol : modified_symbols) { abstract_object_pointert value = diff --git a/src/analyses/variable-sensitivity/variable_sensitivity_domain.h b/src/analyses/variable-sensitivity/variable_sensitivity_domain.h index 421ce44726f7..f2f547060117 100644 --- a/src/analyses/variable-sensitivity/variable_sensitivity_domain.h +++ b/src/analyses/variable-sensitivity/variable_sensitivity_domain.h @@ -172,18 +172,17 @@ class variable_sensitivity_domaint : public ai_domain_baset virtual bool merge(const variable_sensitivity_domaint &b, trace_ptrt from, trace_ptrt to); - /// Perform a context aware merge of the changes that have been applied - /// between function_start and the current state. Anything that has not been - /// modified will be taken from the \p function_call domain. - /// \param function_call: The local of the merge - values from here will be - /// taken if they have not been modified - /// \param function_start: The base of the merge - changes that have been made + /// Merges just the things that have changes between + /// "function_start" and "function_end" into "this". + /// To be used correctly "this" must be derived from the function + /// call site. Anything that is not modified in the function (such + /// as globals) will not be changed. + /// \param function_start: The base of the diff - changes that have been made /// between here and the end will be retained. /// \param function_end: The end of the merge - changes that have been made /// between the start and here will be retained. /// \param ns: The global namespace virtual void merge_three_way_function_return( - const ai_domain_baset &function_call, const ai_domain_baset &function_start, const ai_domain_baset &function_end, const namespacet &ns);