From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42852 invoked by alias); 12 Nov 2015 14:57:43 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 42842 invoked by uid 89); 12 Nov 2015 14:57:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ig0-f174.google.com Received: from mail-ig0-f174.google.com (HELO mail-ig0-f174.google.com) (209.85.213.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 12 Nov 2015 14:57:42 +0000 Received: by igvi2 with SMTP id i2so16054124igv.0 for ; Thu, 12 Nov 2015 06:57:40 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.50.72.51 with SMTP id a19mr15950207igv.21.1447340260280; Thu, 12 Nov 2015 06:57:40 -0800 (PST) Received: by 10.36.209.7 with HTTP; Thu, 12 Nov 2015 06:57:40 -0800 (PST) In-Reply-To: References: Date: Thu, 12 Nov 2015 14:57:00 -0000 Message-ID: Subject: Re: [PATCH] New version of libmpx with new memmove wrapper From: Ilya Enkovich To: Aleksandra Tsvetkova Cc: gcc-patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg01537.txt.bz2 2015-11-05 13:37 GMT+03:00 Aleksandra Tsvetkova : > New version of libmpx was added. There is a new function get_bd() that > allows to get bounds directory. Wrapper for memmove was modified. Now > it moves data and then moves corresponding bounds directly from one > bounds table to another. This approach made moving unaligned pointers > possible. It also makes memmove function faster on sizes bigger than > 64 bytes. +2015-10-27 Tsvetkova Alexandra + + * gcc.target/i386/mpx/memmove.c: New test for __mpx_wrapper_memmove. + Did you test it on different targets? It seems to me this test will fail if you run it on non-MPX target. Please look at mpx-check.h and how other MPX run tests use it. + * mpxrt/mpxrt.c (NUM_L1_BITS): Moved to mpxrt.h. + * mpxrt/mpxrt.c (REG_IP_IDX): Moved to mpxrt.h. + * mpxrt/mpxrt.c (REX_PREFIX): Moved to mpxrt.h. + * mpxrt/mpxrt.c (XSAVE_OFFSET_IN_FPMEM): Moved to mpxrt.h. + * mpxrt/mpxrt.c (MPX_L1_SIZE): Moved to mpxrt.h. No need to repeat file name. + * libmpxwrap/mpx_wrappers.c: Rewrite __mpx_wrapper_memmove to make it faster. You added new functions, types and modified existing function. Make ChangeLog more detailed. --- /dev/null +++ b/libmpx/mpxrt/mpxrt.h @@ -0,0 +1,75 @@ +/* mpxrt.h -*-C++-*- + * + ************************************************************************* + * + * @copyright + * Copyright (C) 2014, 2015, Intel Corporation + * All rights reserved. 2015 only +const uintptr_t MPX_L1_ADDR_MASK = 0xfffff000UL; +const uintptr_t MPX_L2_ADDR_MASK = 0xfffffffcUL; +const uintptr_t MPX_L2_VALID_MASK = 0x00000001UL; Use defines --- a/libmpx/mpxwrap/Makefile.am +++ b/libmpx/mpxwrap/Makefile.am @@ -1,4 +1,5 @@ ALCLOCAL_AMFLAGS = -I .. -I ../config +AM_CPPFLAGS = -I $(top_srcdir) This is not reflected in ChangeLog +/* The mpx_bt_entry struct represents a cell in bounds table. + *lb is the lower bound, *ub is the upper bound, + *p is the stored pointer. */ Bounds and pointer are in lb, ub, p, not in *lb, *ub, *p. Right? +static inline void +alloc_bt (void *ptr) +{ + __asm__ __volatile__ ("bndstx %%bnd0, (%0,%0)"::"r" (ptr):"%bnd0"); +} This should be marked as bnd_legacy. +/* move_bounds function copies N bytes from SRC to DST. Really? + It also copies bounds for all pointers inside. + There are 3 parts of the algorithm: + 1) We copy everything till the end of the first bounds table SRC) SRC is not a bounds table + 2) In loop we copy whole bound tables till the second-last one + 3) Data in the last bounds table is copied separately, after the loop. + If one of bound tables in SRC doesn't exist, + we skip it because there are no pointers. + Depending on the arrangement of SRC and DST we copy from the beginning + or from the end. */ +__attribute__ ((bnd_legacy)) static void * +move_bounds (void *dst, const void *src, size_t n) What is returned value for? +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. Thanks, Ilya