On 12/20/2016 07:16 AM, Richard Biener wrote:
it should be already set as we use visited_ssa as "was it ever on the worklist",
so maybe renaming it would be a good thing as well.
+ if (TREE_CODE (name) == SSA_NAME)
+ {
+ /* If an SSA has already been seen, this may be a loop.
+ Fail conservatively. */
+ if (bitmap_bit_p (visited_ssa, SSA_NAME_VERSION (name)))
+ return false;
so to me "conservative" is returning true, not false.
Agreed.
+ bitmap_set_bit (visited_ssa, SSA_NAME_VERSION (name));
+ worklist.safe_push (name);
but for loops we can just continue and ignore this use. And bitmap_set_bit
returns whether it set a bit, thus
if (bitmap_set_bit (visited_ssa, SSA_NAME_VERSION (name)))
worklist.safe_push (name);
should work?
What if we're in an irreducible region?
In principle the thing is sound but I'd like to be able to pass in a bitmap of
known maybe-undefined/must-defined SSA names to have a cache for
multiple invocations from within a pass (like this unswitching case).
So that means keeping another bitmap for things positively identified as
defined, then saving it for later invocations. So maybe some of my work
+ some of his melded together. (mine computes once for the entire FN
and saves the result, so converting it to on-demand saving the result
shouldn't be terrible).
Only unswitching on conditions that post-dominate the loop entry might be a
simpler solution for the PR in question. OTOH this may disable too much
unswitching in practice.
It might be worth experimentation.
Jeff
