Re: [PATCH] New version of libmpx with new memmove wrapper

[Aleksandra Tsvetkova <astsvetk at gmail dot com>]

gcc/testsuite/ChangeLog
+2015-10-27  Tsvetkova Alexandra  <aleksandra.tsvetkova@intel.com>
+
+ * gcc.target/i386/mpx/memmove.c: New test for __mpx_wrapper_memmove.

libmpx/ChangeLog
+2015-10-28  Tsvetkova Alexandra  <aleksandra.tsvetkova@intel.com>
+
+ * mpxrt/Makefile.am (libmpx_la_LDFLAGS): Add -version-info option.
+ * libmpxwrap/Makefile.am (libmpx_la_LDFLAGS): Likewise + includes fixed.
+ * libmpx/Makefile.in: Regenerate.
+ * mpxrt/Makefile.in: Regenerate.
+ * libmpxwrap/Makefile.in: Regenerate.
+ * mpxrt/libtool-version: New version.
+ * libmpxwrap/libtool-version: Likewise.
+ * mpxrt/libmpx.map: Add new version and a new symbol.
+ * mpxrt/mpxrt.h: New file.
+ * mpxrt/mpxrt.c (NUM_L1_BITS): Moved to mpxrt.h.
+                (REG_IP_IDX): Moved to mpxrt.h.
+                (REX_PREFIX): Moved to mpxrt.h.
+                (XSAVE_OFFSET_IN_FPMEM): Moved to mpxrt.h.
+                (MPX_L1_SIZE): Moved to mpxrt.h.
+ * libmpxwrap/mpx_wrappers.c: Rewrite __mpx_wrapper_memmove
+ to make it faster.
+ New types: mpx_pointer for extraction of indexes from pointer
+           mpx_bt_entry represents a cell in bounds table.
+ New functions: alloc_bt for allocatinn bounds table
+               get_bt to get address of bounds table
+   copy_if_possible and copy_if_possible_from_end move elements
+   of bounds table if we can
+   move_bounds moves bounds just like memmove


All fixed except for:

>>+static inline void
>>+alloc_bt (void *ptr)
>>+{
>>+  __asm__ __volatile__ ("bndstx %%bnd0, (%0,%0)"::"r" (ptr):"%bnd0");
>>+}
>
>This should be marked as bnd_legacy.

It will not work.

> +void *
> +__mpx_wrapper_memmove (void *dst, const void *src, size_t n)
> +{
> +  if (n == 0)
> +    return dst;
> +
> +  __bnd_chk_ptr_bounds (dst, n);
> +  __bnd_chk_ptr_bounds (src, n);
> +
> +  memmove (dst, src, n);
> +  move_bounds (dst, src, n);
> +  return dst;
>  }
>
> You completely remove old algorithm which should be faster on small
> sizes. __mpx_wrapper_memmove should become a dispatcher between old
> and new implementations depending on target (32-bit or 64-bit) and N.
> Since old version performs both data and bounds copy, BD check should
> be moved into __mpx_wrapper_memmove to never call
> it when MPX is disabled.

Even though the old algorithm is faster on small sizes, it should not be used
with the new one because the new one supports unaligned pointers and the
old one does not. Different behavior may cause more problems.

Thanks,
Alexandra.

Attachment: patch_v4.diff
Description: Text document