On Sat, Mar 21, 2015 at 09:33:22PM +1100, Bruce Evans wrote:
> On Sat, 21 Mar 2015, Konstantin Belousov wrote:
> How does the unconditional use of popcntq (in inline asm that is called
> from amd64 pmap) even work?
It is not unconditional. The pmap code does the following:
if ((cpu_feature2 & CPUID2_POPCNT) == 0) {
free = popcnt_pc_map_elem(pc->pc_map[0]);
...
} else {
free = popcntq(pc->pc_map[0]);
...
}
>
> jhb's cleanups apparently missed this inline asm, while previous work
> should not have added it. This inline asm should never have existed,
> since compilers can generate the same code with less-incorrect
> configuration. Correct configuration would be messy. jhb's cleanups
> depend on a compiler predefine (__POPCNT__) to determine if the
> instruction can be used. But this definition is static and is usually
> wrong, since compilers default to plain amd64 which doesn't have the
> instruction. Dynamic configuration would have to use cpuid just to
> determine if the instruction exists.
Exactly, this is what supposed to be done, and is done in the pmap.
It is fine for somebody targeting specific machine to change -march,
but it cannot be used to optimize the generic kernel which is shipped.
>
> >>> Jilles noted that gcc head and 4.9.2 already provides a workaround by
> >>> xoring the dst register. I have some patch for amd64 pmap, see the end
> >>> of the message.
>
> I thought that that couldn't work since it uses the instruction
> unconditionally. But the old code already does that.
>
> >> IIRC, then patch never never uses asm, but intentionally uses the popcount
> >> builtin to avoid complications.
>
> popcntq() in amd64/include/cpufunc.h does use asm, but is missing the
> complications needed for when it the instruction is not available.
> All callers (just 3 in pmap) seem to be broken, since they are also
> missing the complications.
See above.
> > Still, it is weird to provide functions from the sys/types.h namespace,
> > and even more weird to provide such special-purpose function.
>
> Not in the sys/types.h, but in the implementation namespace :-). Since
> no one should depend on the implementation details, we can move the
> details to a better place when one is known. Better places would be
> something like libkern for userland to hold a large collection of
> functions, and a Standard place for single functions. I expect the
> Standard place will be broken as designed for compatibility. Probably
> strings.h alongside ffs().
strings.h sounds less surprising than sys/types.h
>
> The special (inline/asm) functions are in sys/types.h on amd64 are
> currently limited to just 3 bswap functions and some new popcnt functions.
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
