[PATCH] Find constant definition for by-ref argument using dominance information (PR ipa/90401)
Feng Xue OS
fxue@os.amperecomputing.com
Thu Jun 6 08:54:00 GMT 2019
> I don't think your PHI handling works correct. If you have
> agg.part1 = 0;
> if ()
> agg.part2 = 1;
> call (agg);
> then you seem to end up above for agg.part2 because you push that onto the
> worklist below.
It is correct.
o. worklist is used to collect all non-dom virtual operands. agg.part2 is collected
into worklist, after it is taken from worklist, and processed, we find the next dom
virtual operands agg.part1.
[the statement "dom_vuse = vuse"].
o. Execution transfers to the start of main loop, which processes dom virtual
operands, and does overlap analysis between agg.part1 and agg.part2.
[the statement if (!clobber_by_agg_contents_list_p())]
>> + }
>> + append.safe_push (gimple_vuse (stmt));
>> + }
>> + else
>> + {
>> + for (unsigned i = 0; i < gimple_phi_num_args (stmt); ++i)
>> + {
>> + tree phi_arg = gimple_phi_arg_def (stmt, i);
>> +
>> + if (SSA_NAME_IS_DEFAULT_DEF (phi_arg))
>> + goto finish;
>> +
>> + append.safe_push (phi_arg);
> Not sure why you first push to append and then move parts to the
> worklist below - it seems to only complicate code.
It is just a code trick. I do not want to duplicate below codes for both
normal virtual SSA and PHI virtual SSA.
>> + {
>> + if (visited.add (vuse = append[i]))
>> + continue;
>> +
>> + if (SSA_NAME_IS_DEFAULT_DEF (vuse)
>> + || strictly_dominated_by_ssa_p (call_bb, vuse))
>> + {
>> + /* Found a new dom virtual operand, stop going further until
>> + all pending non-dom virtual operands are processed. */
>> + gcc_assert (!dom_vuse);
>> + dom_vuse = vuse;
>> + }
>> + else
>> + worklist.safe_push (vuse);
>> + }
> What you want to do is (probably) use get_continuation_for_phi
> which for a virtual PHI node and a reference computes a
> virtual operand you can continue your processing with. Thus sth
> like
I looked though the function. Yes, it does nearly same thing as this patch requires.
But a minor difference, get_continuation_for_phi() does not translate virtual
operands in back edge. Then, if rewriting with the function, we can not handle the
following case.
/* assume no overlap between agg.part1 and agg.part2 */
__attribute__((pure)) call();
agg.part1 = 0;
for (...)
{
call(agg);
agg.part2 = 1;
}
No sure this restrict is to prevent revisit the start virtual SSA or due to
some other consideration.
Thanks for comments.
Feng
More information about the Gcc-patches
mailing list