> On Thu, Mar 10, 2016 at 5:41 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
> > On Thu, Mar 10, 2016 at 5:24 AM, Richard Biener
> > <richard.guent...@gmail.com> wrote:
> >> On Thu, Mar 10, 2016 at 2:16 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> >>> On Thu, Mar 10, 2016 at 5:01 AM, Richard Biener
> >>> <richard.guent...@gmail.com> wrote:
> >>>> On Thu, Mar 10, 2016 at 1:48 PM, H.J. Lu <hjl.to...@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.
The only CFG change STV makes is via split_edge. But split_edge fixes up
dominance info. I don't see how it may become invalid.
Thanks,
Ilya
>
> --
> H.J.
