On Thu, Jul 30, 2020 at 3:22 PM Martin Liška <mli...@suse.cz> wrote:
>
> On 7/29/20 3:47 PM, Richard Biener wrote:
> > On Tue, Jul 28, 2020 at 12:23 PM Martin Liška <mli...@suse.cz> wrote:
> >>
> >> On 7/27/20 1:31 PM, Richard Biener wrote:
> >>> No, add gro*_exact variants and replace existing ones with this, then 
> >>> switch
> >>> to exact = false for the non-_exact variants.  Or add a exact=false 
> >>> argument
> >>> to all of them and make all existing calls explicitly passing true.
> >>
> >> -EBITLAZY
> >
> > Indeed you are - you're missing to update a lot of vec_safe_grow[_cleared]
> > calls.  Esp. the lto-streamer ones warrant a , true arg since we explicitly
> > streamed the number of elements.
>
> Yep, these should probably use exact allocation.
>
> >
> > How did you select the ones you updated and those you did not?
>
> Based on testing, I only kept changes in selective scheduling which caused
> some ICEs.
>
> (which
> > is why I asked for a boiler-plate replacement of , true first, then making
>
> This step would require to change about 250 places in the source code :(
>
> > the parameter defaulted to false and adjusting those cases you found)
>
> What about rather doing exact = true and changing the few places which don't 
> need
> it (as they make their of growth calculations)?

But that would be inconsistent with the other 'exact' defaulted params in vec.h.

I think your patch points at the valid issue that the usually
defaulted 'exact' is
overridden in non _exact named functions without any way to change that
override.  But then we shouldn't change behavior when correcting that mistake.

Which is why my original suggestion added non-defaulted 'exact' paramters
to those places (to discover all places that need adjustment) and only after
that default it and remove the cases now equal to the defaulted case ...

Richard.

> Martin
>
> >> Anyway, I prepared a patch that adds exact=false for all the grow 
> >> functions.
> >> Apart from selective scheduling, all other consumers seems happy. And I 
> >> removed
> >> the artificial capacity growth calculations at places mentioned by Honza.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> >> Thanks,
> >> Martin
>

Reply via email to