From 3039eee3049c14f6dc803a9bf490b8ee9bb8ec04 Mon Sep 17 00:00:00 2001 From: Olly Betts Date: Sat, 17 Dec 2016 20:42:27 +1300 Subject: [PATCH] Check signed right shift behaviour at compile time We can use a test on a constant expression which should optimise away to just the required version of the code, which means that on platforms which perform sign-extension (pretty much everything current it seems) we don't have to rely on the compiler optimising a portable idiom correctly. (cherry picked from commit f08e5fb4637a46a15d83f8584d5eed44ee3e072f) --- xapian-core/include/xapian/unicode.h | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/xapian-core/include/xapian/unicode.h b/xapian-core/include/xapian/unicode.h index 1995e7fe3..baec5b429 100644 --- a/xapian-core/include/xapian/unicode.h +++ b/xapian-core/include/xapian/unicode.h @@ -268,21 +268,23 @@ namespace Internal { * character from its info. */ inline int get_delta(int info) { - /* It's implementation defined if sign extension happens on right shift - * of a signed int, hence the conditional (hopefully the compiler will - * spot this and optimise it to a sign-extending shift on architectures - * with a suitable instruction). + /* It's implementation defined if sign extension happens when right + * shifting a signed int, although in practice sign extension is what + * most compilers implement. + * + * Some compilers are smart enough to spot common idioms for sign + * extension, but not all (e.g. GCC < 7 doesn't spot the one used in + * the else below), so check what the implementation defined behaviour + * is with a constant conditional which should get optimised away. */ -#ifdef __GNUC__ - // GCC 4.7.1 doesn't optimise the more complex expression down - // (reported as http://gcc.gnu.org/PR55299), but the documented - // behaviour for GCC is that right shift of a signed integer performs - // sign extension: - // http://gcc.gnu.org/onlinedocs/gcc-4.7.2/gcc/Integers-implementation.html - return info >> 8; -#else - return (info >= 0) ? (info >> 8) : (~(~info >> 8)); -#endif + if ((-1 >> 1) == -1) { + // Right shift sign-extends. + return info >> 8; + } else { + // Right shift shifts in zeros, not before and after the shift for + // negative values. + return (info >= 0) ? (info >> 8) : (~(~info >> 8)); + } } } -- 2.11.4.GIT