On Sat, Jul 30, 2016 at 08:18:31PM +0200, René Scharfe wrote:
> The result of st_mult() is the same no matter the order of its
> arguments. It invokes the macro unsigned_mult_overflows(), which
> divides the second parameter by the first one. Pass constants
> first to allow that division to be done already at compile time.
I'm not opposed to this, as it's easy to do (though I suspect new calls
may be introduced that violate it).
But if we really are worried about the performance of st_mult(), I
think:
static inline size_t st_mult(size_t a, size_t b)
{
size_t result;
if (!__builtin_mul_overflow(a, b, &result))
die("whoops!");
return result;
}
is the right direction. I just haven't gotten around to producing a
polished patch.
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 58ac0a5..73d003a 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -541,7 +541,7 @@ void diffcore_rename(struct diff_options *options)
> rename_dst_nr * rename_src_nr, 50, 1);
> }
>
> - mx = xcalloc(st_mult(num_create, NUM_CANDIDATE_PER_DST), sizeof(*mx));
> + mx = xcalloc(st_mult(NUM_CANDIDATE_PER_DST, num_create), sizeof(*mx));
I didn't look at all of the calls, but I wonder if it is a natural
pattern to put the constant second. Since multiplication is
commutative, it would be correct for st_mult() to just flip the order of
arguments it feeds to unsigned_mult_overflows().
That may introduce the same inefficiency in other callsites, but I
wonder if it would be fewer.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html