Skip to content

Commit 515f263

Browse files
committed
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.
1 parent 549558e commit 515f263

5 files changed

+18
-37
lines changed

src/analyses/variable-sensitivity/three_way_merge_abstract_interpreter.cpp

+10-11
Original file line numberDiff line numberDiff line change
@@ -160,22 +160,21 @@ bool ai_three_way_merget::visit_edge_function_call(
160160
*this,
161161
ns);
162162

163-
// TODO : this is probably needed to avoid three_way_merge modifying one of
164-
// its arguments as it goes. A better solution would be to refactor
165-
// merge_three_way_function_return.
166-
const std::unique_ptr<statet> ptr_s_working_copy(
167-
make_temporary_state(s_working));
163+
// The base for the three way merge is the call site
164+
const std::unique_ptr<statet> ptr_call_site_working(
165+
make_temporary_state(get_state(p_call_site)));
166+
auto tmp2 =
167+
dynamic_cast<variable_sensitivity_domaint *>(&(*ptr_call_site_working));
168+
INVARIANT(tmp2 != nullptr, "Three-way merge requires domain support");
169+
variable_sensitivity_domaint &s_call_site_working = *tmp2;
168170

169171
log.progress() << "three way merge... ";
170-
s_working.merge_three_way_function_return(
171-
get_state(p_call_site),
172-
get_state(p_callee_start),
173-
*ptr_s_working_copy,
174-
ns);
172+
s_call_site_working.merge_three_way_function_return(
173+
get_state(p_callee_start), s_working, ns);
175174

176175
log.progress() << "merging... ";
177176
if(
178-
merge(s_working, p_callee_end, p_return_site) ||
177+
merge(s_call_site_working, p_callee_end, p_return_site) ||
179178
(return_step.first == ai_history_baset::step_statust::NEW &&
180179
!s_working.is_bottom()))
181180
{

src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.cpp

+2-13
Original file line numberDiff line numberDiff line change
@@ -438,20 +438,9 @@ bool variable_sensitivity_dependence_domaint::merge(
438438
}
439439

440440
/**
441-
* Perform a context aware merge of the changes that have been applied
442-
* between function_start and the current state. Anything that has not been
443-
* modified will be taken from the \p function_call domain.
444-
*
445-
* \param function_call: The local of the merge - values from here will be
446-
* taken if they have not been modified
447-
* \param function_start: The base of the merge - changes that have been made
448-
* between here and the end will be retained.
449-
* \param function_end: The end of the merge - changes that have been made
450-
/// between the start and here will be retained.
451-
* \param ns: The global namespace
441+
* \copydoc variable_sensitivity_domaint::merge_three_way_function_return
452442
*/
453443
void variable_sensitivity_dependence_domaint::merge_three_way_function_return(
454-
const ai_domain_baset &function_call,
455444
const ai_domain_baset &function_start,
456445
const ai_domain_baset &function_end,
457446
const namespacet &ns)
@@ -462,7 +451,7 @@ void variable_sensitivity_dependence_domaint::merge_three_way_function_return(
462451
// the three way merge is that the underlying variable sensitivity domain
463452
// does its three way merge.
464453
variable_sensitivity_domaint::merge_three_way_function_return(
465-
function_call, function_start, function_end, ns);
454+
function_start, function_end, ns);
466455
}
467456

468457
/**

src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.h

-1
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ class variable_sensitivity_dependence_domaint
132132
trace_ptrt to) override;
133133

134134
void merge_three_way_function_return(
135-
const ai_domain_baset &function_call,
136135
const ai_domain_baset &function_start,
137136
const ai_domain_baset &function_end,
138137
const namespacet &ns) override;

src/analyses/variable-sensitivity/variable_sensitivity_domain.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -417,14 +417,10 @@ bool variable_sensitivity_domaint::ignore_function_call_transform(
417417
}
418418

419419
void variable_sensitivity_domaint::merge_three_way_function_return(
420-
const ai_domain_baset &function_call,
421420
const ai_domain_baset &function_start,
422421
const ai_domain_baset &function_end,
423422
const namespacet &ns)
424423
{
425-
const variable_sensitivity_domaint &cast_function_call =
426-
static_cast<const variable_sensitivity_domaint &>(function_call);
427-
428424
const variable_sensitivity_domaint &cast_function_start =
429425
static_cast<const variable_sensitivity_domaint &>(function_start);
430426

@@ -443,7 +439,6 @@ void variable_sensitivity_domaint::merge_three_way_function_return(
443439
std::back_inserter(modified_symbols),
444440
[&ns](const irep_idt &id) { return ns.lookup(id).symbol_expr(); });
445441

446-
abstract_state = cast_function_call.abstract_state;
447442
for(const auto &symbol : modified_symbols)
448443
{
449444
abstract_object_pointert value =

src/analyses/variable-sensitivity/variable_sensitivity_domain.h

+6-7
Original file line numberDiff line numberDiff line change
@@ -172,18 +172,17 @@ class variable_sensitivity_domaint : public ai_domain_baset
172172
virtual bool
173173
merge(const variable_sensitivity_domaint &b, trace_ptrt from, trace_ptrt to);
174174

175-
/// Perform a context aware merge of the changes that have been applied
176-
/// between function_start and the current state. Anything that has not been
177-
/// modified will be taken from the \p function_call domain.
178-
/// \param function_call: The local of the merge - values from here will be
179-
/// taken if they have not been modified
180-
/// \param function_start: The base of the merge - changes that have been made
175+
/// Merges just the things that have changes between
176+
/// "function_start" and "function_end" into "this".
177+
/// To be used correctly "this" must be derived from the function
178+
/// call site. Anything that is not modified in the function (such
179+
/// as globals) will not be changed.
180+
/// \param function_start: The base of the diff - changes that have been made
181181
/// between here and the end will be retained.
182182
/// \param function_end: The end of the merge - changes that have been made
183183
/// between the start and here will be retained.
184184
/// \param ns: The global namespace
185185
virtual void merge_three_way_function_return(
186-
const ai_domain_baset &function_call,
187186
const ai_domain_baset &function_start,
188187
const ai_domain_baset &function_end,
189188
const namespacet &ns);

0 commit comments

Comments
 (0)