On December 20, 2016 4:50:25 PM GMT+01:00, Jeff Law <l...@redhat.com> wrote:
>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?
Handling all back edges by deferring to their defs should work. At least I
can't see how it would not.
>
>>
>> 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.
We eventually need a tristate here for maximum caching. And as the result
depends on the dominating BB of the postdom region the savings might be
questionable.
> 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
