On Thu, 9 Jul 2026, Michal Jireš wrote:

> 
> On Thu Jul 9, 2026 at 3:26 PM CEST, Richard Biener <[email protected]> wrote:
> > On Thu, 9 Jul 2026, Michal Jireš wrote:
> >
> >> 
> >> On Thu Jul 9, 2026 at 2:45 PM CEST, Richard Biener <[email protected]> 
> >> wrote:
> >> > On Thu, 9 Jul 2026, Michal Jires wrote:
> >> >
> >> >> Cache partitioning asserts that {min,max}_partition_size parameter
> >> >> cannot be 0 to prevent later divisions by zero. This is needlessly
> >> >> strict, we can clamp the value to 1 to get reasonable/expected behavior.
> >> >
> >> > Can you instead add IntegerRage(1, 2147483647) to the params in
> >> > param.opt?
> >> >
> >> 
> >> Both {min,max}_partition_size currently work with 0 with the default
> >> balanced partitioning, and likely someone uses it.
> >> 
> >> And both values make some sense:
> >> min_partition_size=0 -> there is no minimal partition size
> >> max_partition_size=0 -> put every function into its own partition
> >> 
> >> Both seem to be an expected outcome of approaching/reaching 0.
> >
> > But the assert now triggers for all partitioning algorithms since
> > you touch partitioner_base?
> 
> partitioner_base can be used for new partitionings, but partitioner_base
> is currently used only for balanced partitioning.
> 
> > That said, I'm fine with changing
> > the behavior of --param min_partition_size, the exteme you
> > mention should be -flto-partition=max, the other (always use
> > lto-partitions number of partitions if possible) isn't directly
> > accessible.
> Both can be achieved by specifying {min,max}_partition_size=1.
> But I don't see what is the benefit of forbidding this seemingly
> reasonable value.
> 
> >
> > Btw, why does std::max (...) without the explicit type not work?
> >
> std::max needs both arguments to have the same type. I could use 1l or 
> 1ll to specify the type, but both are not guaranteed to be int64_t.

I see.

The original patch is OK then.
Richard.

Reply via email to