Skip to content

Commit

Permalink
Refactor three-way-merge so that it makes more sense
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
martin-cs committed Sep 28, 2023
1 parent 76ff9e3 commit 2092f2b
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<statet> ptr_s_working_copy(
make_temporary_state(s_working));
// The base for the three way merge is the call site
const std::unique_ptr<statet> ptr_call_site_working(
make_temporary_state(get_state(p_call_site)));
auto tmp2 =
dynamic_cast<variable_sensitivity_domaint *>(&(*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()))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<const variable_sensitivity_domaint &>(function_call);

const variable_sensitivity_domaint &cast_function_start =
static_cast<const variable_sensitivity_domaint &>(function_start);

Expand All @@ -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 =
Expand Down
13 changes: 6 additions & 7 deletions src/analyses/variable-sensitivity/variable_sensitivity_domain.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 2092f2b

Please sign in to comment.