Shouldn't convert_scalars_to_vector call free_dominance_info?

[H.J. Lu]

On Thu, Mar 10, 2016 at 5:41 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Mar 10, 2016 at 5:24 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> 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.
>
> Remove that assert doesn't solve all problems.  Now I got
>
> 0x98483a verify_dominators(cdi_direction)
>         /export/gnu/import/git/sources/gcc/gcc/dominance.c:1038
> 0x982591 checking_verify_dominators
>         /export/gnu/import/git/sources/gcc/gcc/dominance.h:71
> 0x983d27 calculate_dominance_info(cdi_direction)
>         /export/gnu/import/git/sources/gcc/gcc/dominance.c:664
> 0x1742a20 fwprop_init
>         /export/gnu/import/git/sources/gcc/gcc/fwprop.c:1393
> 0x1742cf8 fwprop_addr
>         /export/gnu/import/git/sources/gcc/gcc/fwprop.c:1511
> 0x1742e60 execute
>         /export/gnu/import/git/sources/gcc/gcc/fwprop.c:1557
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <http://gcc.gnu.org/bugs.html> for instructions.
> Makefile:612: recipe for target 'bid128_fma.o' failed
> make[2]: *** [bid128_fma.o] Error 1
>
> if convert_scalars_to_vector doesn't call
>
> free_dominance_info (CDI_DOMINATORS);

Since convert_scalars_to_vector may add instructions, dominance
info is no longer up to date.

-- 
H.J.