[PATCH] fix asm when strict ISO c and ensure dix-config.h is included first
Matt Turner
mattst88 at gmail.com
Wed Nov 18 16:59:52 PST 2015
On Mon, Nov 16, 2015 at 10:03 PM, Richard PALO <richard at netbsd.org> wrote:
> From: Richard PALO <richard at NetBSD.org>
>
> The following is for the source found in the glamor subdirectory:
>
> Define a macro 'asm' for the case of a strict ISO gnu c compatible compiler,
> (NB this may need to be revised in case of being compiled in by c++)
>
> Also, ensure that dix-config.h is effectively the first included file in
> various glamor modules such that feature tests (at least on SunOS i386)
> is correctly instrumented (such as _LARGEFILE_SOURCE/_FILE_OFFSET_BITS
> as well as POSIX profile and other extensions).
>
> Since glamor_priv.h correctly includes dix-config.h, the easiest way is
> to simply adjust its position in the affected .c files.
>
> Signed-off-by: Richard PALO <richard at NetBSD.org>
> ---
> glamor/glamor.c | 2 +-
> glamor/glamor_composite_glyphs.c | 2 +-
> glamor/glamor_core.c | 3 +--
> glamor/glamor_fbo.c | 3 +--
> glamor/glamor_largepixmap.c | 3 +--
> glamor/glamor_picture.c | 2 +-
> glamor/glamor_pixmap.c | 2 +-
> glamor/glamor_utils.h | 4 ++++
> 8 files changed, 11 insertions(+), 10 deletions(-)
This is all because of
static inline unsigned long
__fls(unsigned long x)
{
asm("bsr %1,%0":"=r"(x)
: "rm"(x));
return x;
}
There's no reason to use inline assembly there and in fact it's likely
harmful. Also, bsr has unspecified behavior for an input of zero, so
this implementation doesn't even match the non-assembly
implementation.
Just change the implementation to
return n == 0 ? 0 : 32 - __builtin_clz(n);
and get rid of the other definition of __fls() immediately below.
__builtin_ctz has been supported by gcc since v3.4 so it should be
safe to use anywhere gcc inline assembly is safe to use.
More information about the xorg-devel
mailing list