[PATCH v2 1/3] string: add find_ne_all to string-fza.h

[Wilco Dijkstra]

Hi Matt,

Using HAVE_BITOPTS_WORKING in target specific implementations
won't work since they already use an incompatible mask representation.
So it is best to keep it only in the generic implementation.

And since no target needs it, we should probably just get rid of it (and
instead add a string-fzi.h for targets that may need it in the future).


diff --git ./sysdeps/alpha/string-fza.h ./sysdeps/alpha/string-fza.h
 
+/* Identify bytes that are not equal between X1 and X2.  */
+static __always_inline find_t
+find_ne_all (op_t x1, op_t x2)
+{
+  return find_zero_all (x1 ^ x2) ^ 0xff;
+}

OK

+++ ./sysdeps/arm/armv6t2/string-fza.h
@@ -19,6 +19,7 @@
 #ifndef _STRING_FZA_H
 #define _STRING_FZA_H 1
 
>+#include <string-bitops.h>

We never need this since HAVE_BITOPTS_WORKING cannot be false for
Armv6t2 (and it can't work if you force it).


+/* Identify bytes that are not equal between X1 and X2.  */
+static __always_inline find_t
+find_ne_all (op_t x1, op_t x2)
+{
+#if HAVE_BITOPTS_WORKING
+  /* The difference need not be reduced; see the generic string-fza.h.  */
+  return x1 ^ x2;

Just keep this, get rid of the rest.

+#else
+  op_t ones = repeat_bytes (0x01);
+  return find_zero_all (x1 ^ x2) ^ ones;

This is incorrect since we set bit 0 of each byte while the generic code assumes
it is bit 7 if !HAVE_BITOPTS_WORKING.


--- ./sysdeps/generic/string-fza.h

+/* With similar caveats, identify bytes that are not equal between X1
+   and X2.  */
+static __always_inline find_t
+find_ne_all (op_t x1, op_t x2)
+{
+#if HAVE_BITOPTS_WORKING
+  /* index_first and index_last only need to know which byte holds the first
+     or the last set bit, so the difference does not have to be reduced to
+     one bit per byte.  The fallback ctzb and clzb do require the reduced
+     form, as does any target whose own index_first tests a fixed bit of
+     each byte; both clear HAVE_BITOPTS_WORKING.  */
+  return x1 ^ x2;
+#else
+  op_t m = repeat_bytes (0x7f);
+  op_t ne = x1 ^ x2;
+  return (((ne & m) + m) | ne) & ~m;
+#endif
+}

OK

+++ ./sysdeps/powerpc/string-fza.h

+# include <string-bitops.h>

Same issue as for Arm - it cannot ever work for !HAVE_BITOPTS_WORKING.

+static __always_inline find_t
+find_ne_all (op_t x1, op_t x2)
+{
+#if HAVE_BITOPTS_WORKING
+  /* The difference need not be reduced; see the generic string-fza.h.  */
+  return x1 ^ x2;

Just keep this part and get rid of the rest.


+++ ./sysdeps/riscv/string-bitops.h

+/* Without the bitmap extensions string-fzi.h defines its own index_first
+   and index_last, which test bit 7 of each byte, and the stdbit.h routines
+   are not used at all.  Clear this so that the generic string-fza.h, which
+   string-fza.h falls back to in that case, reduces its masks to one bit per
+   byte.  */
+#if defined __riscv_zbb || defined __riscv_xtheadbb
+# define HAVE_BITOPTS_WORKING 1
+#else
+# define HAVE_BITOPTS_WORKING 0
+#endif

It's best to update the riscv/string-fzi.h implementation to check all 8 bits.


+++ ./sysdeps/riscv/string-fza.h

+/* Identify bytes that are not equal between X1 and X2.  */
+static __always_inline find_t
+find_ne_all (op_t x1, op_t x2)
+{
+  return ~find_eq_all (x1, x2);
+}

This should just be x1 ^ x2 like Power and Arm.

Cheers,
Wilco