Re: Shouldn't convert_scalars_to_vector call free_dominance_info?

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Mar 10, 2016 at 2:16 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Mar 10, 2016 at 5:01 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Thu, Mar 10, 2016 at 1:48 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> convert_scalars_to_vector in i386.c calls
>>>
>>> calculate_dominance_info (CDI_DOMINATORS);
>>>
>>> Shouldn't it call
>>>
>>> free_dominance_info (CDI_DOMINATORS);
>>>
>>> after it is done like other places where calculate_dominance_info is used?
>>
>> Only if it invalidates it.   Other than post-dominator info dominator
>> info is kept
>> up-to-date between passes.
>>
>>> When I extend the STV pass to 64-bit and put the 64-bit STV pass before
>>> the CSE pass, I run into
>>>
>>> gcc_assert (!dom_info_available_p (CDI_DOMINATORS));
>>
>> That looks like a bogus assert to me.  Which one is it?  The one in
>> cfgcleanup.c?
>> In that case something else should have freed it (the thing that
>> doesn't preserve it)
>> or the condition the assert is guarded is "incorrect".
>
> cleanup_cfg has
>
>  /* ???  We probably do this way too often.  */
>   if (current_loops
>       && (changed
>           || (mode & CLEANUP_CFG_CHANGED)))
>     {
>       timevar_push (TV_REPAIR_LOOPS);
>       /* The above doesn't preserve dominance info if available.  */
>       gcc_assert (!dom_info_available_p (CDI_DOMINATORS));
>       calculate_dominance_info (CDI_DOMINATORS);
>       fix_loop_structure (NULL);
>       free_dominance_info (CDI_DOMINATORS);
>       timevar_pop (TV_REPAIR_LOOPS);
>     }
>
> Should assert be removed?

I think these days we can safely remove it as calculate_dominance_info
will ICE if dom info is said to be present but is incorrect.  I put in
the assert
to catch this case before we had that.

Richard.

> --
> H.J.