Re: [PATCH] Remove some divide instructions

From: Linus Torvalds
Date: Wed Oct 27 2004 - 19:20:00 EST




On Wed, 27 Oct 2004, Zachary Amsden wrote:
>
> Backed out everything but i386 and generic. For the generic version, I
> compiled and tested this code outside of the kernel. Actually, I found
> that at least for my tool chain, the generic version
>
> +# define do_div(n,base) ({ \
> + uint32_t __rem; \
> + if (__builtin_constant_p(base) && !((base) & ((base)-1))) { \
> + __rem = ((uint64_t)(n)) % (base); \
> + (n) = ((uint64_t)(n)) / (base); \
> + } else { \
> + uint32_t __base = (base); \
> + __rem = ((uint64_t)(n)) % __base; \
> + (n) = ((uint64_t)(n)) / __base; \
> + } \
> + __rem;
>
> Does indeed generate different code for the constant case - without it,
> due to the assignment to __base, the shift / mask optimization does not
> take place.

Oh, damn. That's a separate issue, apparently, and there you just use
"__builtin_constant_p()" as a way to check that there are no side effects
on "base".

Might as well drop the check for the value, since it doesn't matter.

Alternatively, we could just document the fact that neither "base" nor "n"
are normal arguments to a function. After all, we already _do_ change "n",
and the strange calling convention is already documented as nothing but a
sick hack to make architectures able to use inline assembly efficiently.

I could add a sparse check for "no side effects", if anybody cares (so
that you could do

__builtin_warning(
!__builtin_nosideeffects(base),
"expression has side effects");

in macros like these.. Sparse already has the logic internally..

Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/