Message ID | 20170629173030.GA25414@intel.com |
---|---|
State | Superseded |
Headers |
Received: (qmail 56149 invoked by alias); 29 Jun 2017 17:30:51 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 55832 invoked by uid 89); 29 Jun 2017 17:30:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, NO_DNS_FOR_FROM, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy=increases X-HELO: mga02.intel.com X-ExtLoop1: 1 Date: Thu, 29 Jun 2017 10:30:31 -0700 From: "H.J. Lu" <hongjiu.lu@intel.com> To: GNU C Library <libc-alpha@sourceware.org> Subject: [PATCH] i386: Increase MALLOC_ALIGNMENT to 16 [BZ #21120] Message-ID: <20170629173030.GA25414@intel.com> Reply-To: "H.J. Lu" <hjl.tools@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.8.0 (2017-02-23) |
Commit Message
Lu, Hongjiu
June 29, 2017, 5:30 p.m. UTC
GCC 7 changed the definition of max_align_t on i386: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2 As a result, glibc malloc no longer returns memory blocks which are as aligned as max_align_t requires. This causes malloc/tst-malloc-thread-fail to fail with an error like this one: error: allocation function 0, size 144 not aligned to 16 This patch increases the malloc alignment to 16 for i386. Tested on i386 with GCC 7 and on x86-64. OK for master? H.J. --- [BZ #21120] * sysdeps/generic/malloc-alignment.h: New file. * sysdeps/i386/malloc-alignment.h: Likewise. * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>. --- sysdeps/generic/malloc-alignment.h | 24 ++++++++++++++++++++++++ sysdeps/generic/malloc-machine.h | 1 + sysdeps/i386/malloc-alignment.h | 24 ++++++++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 sysdeps/generic/malloc-alignment.h create mode 100644 sysdeps/i386/malloc-alignment.h
Comments
On 06/29/2017 01:30 PM, H.J. Lu wrote: > GCC 7 changed the definition of max_align_t on i386: > > https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2 > > As a result, glibc malloc no longer returns memory blocks which are as > aligned as max_align_t requires. > > This causes malloc/tst-malloc-thread-fail to fail with an error like this > one: > > error: allocation function 0, size 144 not aligned to 16 > > This patch increases the malloc alignment to 16 for i386. > > Tested on i386 with GCC 7 and on x86-64. OK for master? > > H.J. > --- > [BZ #21120] > * sysdeps/generic/malloc-alignment.h: New file. > * sysdeps/i386/malloc-alignment.h: Likewise. > * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>. Please use malloc-machine.h which was the previous header that provided machine-dependent malloc definitions. That way we remain consistent across releases and make it easier to backport such changes without adding a new header. OK with that change.
On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote: > On 06/29/2017 01:30 PM, H.J. Lu wrote: >> GCC 7 changed the definition of max_align_t on i386: >> >> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2 >> >> As a result, glibc malloc no longer returns memory blocks which are as >> aligned as max_align_t requires. >> >> This causes malloc/tst-malloc-thread-fail to fail with an error like this >> one: >> >> error: allocation function 0, size 144 not aligned to 16 >> >> This patch increases the malloc alignment to 16 for i386. >> >> Tested on i386 with GCC 7 and on x86-64. OK for master? >> >> H.J. >> --- >> [BZ #21120] >> * sysdeps/generic/malloc-alignment.h: New file. >> * sysdeps/i386/malloc-alignment.h: Likewise. >> * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>. > > Please use malloc-machine.h which was the previous header that provided > machine-dependent malloc definitions. That way we remain consistent across > releases and make it easier to backport such changes without adding a new > header. It won't work too well for Hurd since we have ./sysdeps/generic/malloc-machine.h ./sysdeps/mach/hurd/malloc-machine.h ./sysdeps/nptl/malloc-machine.h What will Hurd/i386 get? malloc-alignment.h handles it automatically. > OK with that change. > > -- > Cheers, > Carlos.
On 06/29/2017 02:06 PM, H.J. Lu wrote: > On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote: >> On 06/29/2017 01:30 PM, H.J. Lu wrote: >>> GCC 7 changed the definition of max_align_t on i386: >>> >>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2 >>> >>> As a result, glibc malloc no longer returns memory blocks which are as >>> aligned as max_align_t requires. >>> >>> This causes malloc/tst-malloc-thread-fail to fail with an error like this >>> one: >>> >>> error: allocation function 0, size 144 not aligned to 16 >>> >>> This patch increases the malloc alignment to 16 for i386. >>> >>> Tested on i386 with GCC 7 and on x86-64. OK for master? >>> >>> H.J. >>> --- >>> [BZ #21120] >>> * sysdeps/generic/malloc-alignment.h: New file. >>> * sysdeps/i386/malloc-alignment.h: Likewise. >>> * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>. >> >> Please use malloc-machine.h which was the previous header that provided >> machine-dependent malloc definitions. That way we remain consistent across >> releases and make it easier to backport such changes without adding a new >> header. > > It won't work too well for Hurd since we have > > ./sysdeps/generic/malloc-machine.h > ./sysdeps/mach/hurd/malloc-machine.h > ./sysdeps/nptl/malloc-machine.h > > What will Hurd/i386 get? malloc-alignment.h handles it automatically. If your patch made Hurd/i386 use MALLOC_ALIGNMENT of 16 then a new patch using malloc-machine.h would set MALLOC_ALIGNMENT to 16 in systeps/mach/hurd/malloc-machine.h?
On Thu, Jun 29, 2017 at 11:11 AM, Carlos O'Donell <carlos@redhat.com> wrote: > On 06/29/2017 02:06 PM, H.J. Lu wrote: >> On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote: >>> On 06/29/2017 01:30 PM, H.J. Lu wrote: >>>> GCC 7 changed the definition of max_align_t on i386: >>>> >>>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2 >>>> >>>> As a result, glibc malloc no longer returns memory blocks which are as >>>> aligned as max_align_t requires. >>>> >>>> This causes malloc/tst-malloc-thread-fail to fail with an error like this >>>> one: >>>> >>>> error: allocation function 0, size 144 not aligned to 16 >>>> >>>> This patch increases the malloc alignment to 16 for i386. >>>> >>>> Tested on i386 with GCC 7 and on x86-64. OK for master? >>>> >>>> H.J. >>>> --- >>>> [BZ #21120] >>>> * sysdeps/generic/malloc-alignment.h: New file. >>>> * sysdeps/i386/malloc-alignment.h: Likewise. >>>> * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>. >>> >>> Please use malloc-machine.h which was the previous header that provided >>> machine-dependent malloc definitions. That way we remain consistent across >>> releases and make it easier to backport such changes without adding a new >>> header. >> >> It won't work too well for Hurd since we have >> >> ./sysdeps/generic/malloc-machine.h >> ./sysdeps/mach/hurd/malloc-machine.h >> ./sysdeps/nptl/malloc-machine.h >> >> What will Hurd/i386 get? malloc-alignment.h handles it automatically. > > If your patch made Hurd/i386 use MALLOC_ALIGNMENT of 16 then a new patch > using malloc-machine.h would set MALLOC_ALIGNMENT to 16 in > systeps/mach/hurd/malloc-machine.h? > This assumes that mach/hurd == i386. Also I don't like define MALLOC_ALIGNMENT to 16 for i386 in 2 different places. Since ./sysdeps/generic/malloc-machine.h ./sysdeps/mach/hurd/malloc-machine.h ./sysdeps/nptl/malloc-machine.h malloc-machine.h is not pure processor specific. It is also OS specific. I prefer to define MALLOC_ALIGNMENT to 16 for i386 in a processor specific header file.
On 06/29/2017 02:43 PM, H.J. Lu wrote: > On Thu, Jun 29, 2017 at 11:11 AM, Carlos O'Donell <carlos@redhat.com> wrote: >> On 06/29/2017 02:06 PM, H.J. Lu wrote: >>> On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote: >>>> On 06/29/2017 01:30 PM, H.J. Lu wrote: >>>>> GCC 7 changed the definition of max_align_t on i386: >>>>> >>>>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2 >>>>> >>>>> As a result, glibc malloc no longer returns memory blocks which are as >>>>> aligned as max_align_t requires. >>>>> >>>>> This causes malloc/tst-malloc-thread-fail to fail with an error like this >>>>> one: >>>>> >>>>> error: allocation function 0, size 144 not aligned to 16 >>>>> >>>>> This patch increases the malloc alignment to 16 for i386. >>>>> >>>>> Tested on i386 with GCC 7 and on x86-64. OK for master? >>>>> >>>>> H.J. >>>>> --- >>>>> [BZ #21120] >>>>> * sysdeps/generic/malloc-alignment.h: New file. >>>>> * sysdeps/i386/malloc-alignment.h: Likewise. >>>>> * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>. >>>> >>>> Please use malloc-machine.h which was the previous header that provided >>>> machine-dependent malloc definitions. That way we remain consistent across >>>> releases and make it easier to backport such changes without adding a new >>>> header. >>> >>> It won't work too well for Hurd since we have >>> >>> ./sysdeps/generic/malloc-machine.h >>> ./sysdeps/mach/hurd/malloc-machine.h >>> ./sysdeps/nptl/malloc-machine.h >>> >>> What will Hurd/i386 get? malloc-alignment.h handles it automatically. >> >> If your patch made Hurd/i386 use MALLOC_ALIGNMENT of 16 then a new patch >> using malloc-machine.h would set MALLOC_ALIGNMENT to 16 in >> systeps/mach/hurd/malloc-machine.h? >> > > This assumes that mach/hurd == i386. Also I don't like define > MALLOC_ALIGNMENT to 16 for i386 in 2 different places. > Since > > ./sysdeps/generic/malloc-machine.h > ./sysdeps/mach/hurd/malloc-machine.h > ./sysdeps/nptl/malloc-machine.h > > malloc-machine.h is not pure processor specific. It is also OS > specific. I prefer to define MALLOC_ALIGNMENT to 16 for i386 > in a processor specific header file. Add a sysdeps/mach/hurd/i386, which is an os/machine directory. Similarly for i386?
On Thu, Jun 29, 2017 at 1:03 PM, Carlos O'Donell <carlos@redhat.com> wrote: > On 06/29/2017 02:43 PM, H.J. Lu wrote: >> On Thu, Jun 29, 2017 at 11:11 AM, Carlos O'Donell <carlos@redhat.com> wrote: >>> On 06/29/2017 02:06 PM, H.J. Lu wrote: >>>> On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote: >>>>> On 06/29/2017 01:30 PM, H.J. Lu wrote: >>>>>> GCC 7 changed the definition of max_align_t on i386: >>>>>> >>>>>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2 >>>>>> >>>>>> As a result, glibc malloc no longer returns memory blocks which are as >>>>>> aligned as max_align_t requires. >>>>>> >>>>>> This causes malloc/tst-malloc-thread-fail to fail with an error like this >>>>>> one: >>>>>> >>>>>> error: allocation function 0, size 144 not aligned to 16 >>>>>> >>>>>> This patch increases the malloc alignment to 16 for i386. >>>>>> >>>>>> Tested on i386 with GCC 7 and on x86-64. OK for master? >>>>>> >>>>>> H.J. >>>>>> --- >>>>>> [BZ #21120] >>>>>> * sysdeps/generic/malloc-alignment.h: New file. >>>>>> * sysdeps/i386/malloc-alignment.h: Likewise. >>>>>> * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>. >>>>> >>>>> Please use malloc-machine.h which was the previous header that provided >>>>> machine-dependent malloc definitions. That way we remain consistent across >>>>> releases and make it easier to backport such changes without adding a new >>>>> header. >>>> >>>> It won't work too well for Hurd since we have >>>> >>>> ./sysdeps/generic/malloc-machine.h >>>> ./sysdeps/mach/hurd/malloc-machine.h >>>> ./sysdeps/nptl/malloc-machine.h >>>> >>>> What will Hurd/i386 get? malloc-alignment.h handles it automatically. >>> >>> If your patch made Hurd/i386 use MALLOC_ALIGNMENT of 16 then a new patch >>> using malloc-machine.h would set MALLOC_ALIGNMENT to 16 in >>> systeps/mach/hurd/malloc-machine.h? >>> >> >> This assumes that mach/hurd == i386. Also I don't like define >> MALLOC_ALIGNMENT to 16 for i386 in 2 different places. >> Since >> >> ./sysdeps/generic/malloc-machine.h >> ./sysdeps/mach/hurd/malloc-machine.h >> ./sysdeps/nptl/malloc-machine.h >> >> malloc-machine.h is not pure processor specific. It is also OS >> specific. I prefer to define MALLOC_ALIGNMENT to 16 for i386 >> in a processor specific header file. > > Add a sysdeps/mach/hurd/i386, which is an os/machine directory. > > Similarly for i386? > Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT? 1. generic i386. 2. nptl i386. 3. hurd i386. That is very weird.
On Thu, Jun 29, 2017 at 1:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jun 29, 2017 at 1:03 PM, Carlos O'Donell <carlos@redhat.com> wrote: >> On 06/29/2017 02:43 PM, H.J. Lu wrote: >>> On Thu, Jun 29, 2017 at 11:11 AM, Carlos O'Donell <carlos@redhat.com> wrote: >>>> On 06/29/2017 02:06 PM, H.J. Lu wrote: >>>>> On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote: >>>>>> On 06/29/2017 01:30 PM, H.J. Lu wrote: >>>>>>> GCC 7 changed the definition of max_align_t on i386: >>>>>>> >>>>>>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2 >>>>>>> >>>>>>> As a result, glibc malloc no longer returns memory blocks which are as >>>>>>> aligned as max_align_t requires. >>>>>>> >>>>>>> This causes malloc/tst-malloc-thread-fail to fail with an error like this >>>>>>> one: >>>>>>> >>>>>>> error: allocation function 0, size 144 not aligned to 16 >>>>>>> >>>>>>> This patch increases the malloc alignment to 16 for i386. >>>>>>> >>>>>>> Tested on i386 with GCC 7 and on x86-64. OK for master? >>>>>>> >>>>>>> H.J. >>>>>>> --- >>>>>>> [BZ #21120] >>>>>>> * sysdeps/generic/malloc-alignment.h: New file. >>>>>>> * sysdeps/i386/malloc-alignment.h: Likewise. >>>>>>> * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>. >>>>>> >>>>>> Please use malloc-machine.h which was the previous header that provided >>>>>> machine-dependent malloc definitions. That way we remain consistent across >>>>>> releases and make it easier to backport such changes without adding a new >>>>>> header. >>>>> >>>>> It won't work too well for Hurd since we have >>>>> >>>>> ./sysdeps/generic/malloc-machine.h >>>>> ./sysdeps/mach/hurd/malloc-machine.h >>>>> ./sysdeps/nptl/malloc-machine.h >>>>> >>>>> What will Hurd/i386 get? malloc-alignment.h handles it automatically. >>>> >>>> If your patch made Hurd/i386 use MALLOC_ALIGNMENT of 16 then a new patch >>>> using malloc-machine.h would set MALLOC_ALIGNMENT to 16 in >>>> systeps/mach/hurd/malloc-machine.h? >>>> >>> >>> This assumes that mach/hurd == i386. Also I don't like define >>> MALLOC_ALIGNMENT to 16 for i386 in 2 different places. >>> Since >>> >>> ./sysdeps/generic/malloc-machine.h >>> ./sysdeps/mach/hurd/malloc-machine.h >>> ./sysdeps/nptl/malloc-machine.h >>> >>> malloc-machine.h is not pure processor specific. It is also OS >>> specific. I prefer to define MALLOC_ALIGNMENT to 16 for i386 >>> in a processor specific header file. >> >> Add a sysdeps/mach/hurd/i386, which is an os/machine directory. >> >> Similarly for i386? >> > > Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT? > > 1. generic i386. > 2. nptl i386. > 3. hurd i386. > > That is very weird. malloc-machine.h is an internal header and was never installed. Adding another internal header makes no differences for backport in this case. FWIW, I backported my malloc-alignment.h patch to glibc 2.25: https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/hjl/pr21120/2.25 and 2.24: https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/hjl/pr21120/2.24 I built and checked them for i686 with GCC 7. The biggest challenge is to support GCC 7, not the header file itself.
On 06/30/2017 07:54 AM, H.J. Lu wrote: > On Thu, Jun 29, 2017 at 1:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT? >> >> 1. generic i386. >> 2. nptl i386. >> 3. hurd i386. >> >> That is very weird. I hope my explanation below makes the difference clearer. > I built and checked them for i686 with GCC 7. The biggest challenge is to > support GCC 7, not the header file itself. We have an existing malloc-machine.h for this purpose. The design question is: (a) Should we add a new file malloc-alignment.h to specify a new parameter. - Adds new file malloc-alignment.h. - 2 new files. (b) Should we use existing malloc-machine.h to include the new parameter? - Extends existing malloc-machine.h to new sysdep dirs. - 3 new files. You are arguing for (a) and I argue for (b). Use 1 less file but require a new header. Use 1 more file but use existing headers. I argue that it's simpler and easier for developers if we avoid additional headers and use existing headers. I'm open hear a comment from someone else.
On Fri, Jun 30, 2017 at 6:05 AM, Carlos O'Donell <carlos@redhat.com> wrote: > On 06/30/2017 07:54 AM, H.J. Lu wrote: >> On Thu, Jun 29, 2017 at 1:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT? >>> >>> 1. generic i386. >>> 2. nptl i386. >>> 3. hurd i386. >>> >>> That is very weird. > > I hope my explanation below makes the difference clearer. > >> I built and checked them for i686 with GCC 7. The biggest challenge is to >> support GCC 7, not the header file itself. > > We have an existing malloc-machine.h for this purpose. > > The design question is: > > (a) Should we add a new file malloc-alignment.h to specify a new parameter. > - Adds new file malloc-alignment.h. > - 2 new files. And define MALLOC_ALIGNMENT for i386 in one place. > > (b) Should we use existing malloc-machine.h to include the new parameter? > - Extends existing malloc-machine.h to new sysdep dirs. > - 3 new files. And define MALLOC_ALIGNMENT for i386 in 3 difference places. If we need to support a new OS or threading library, we need to define MALLOC_ALIGNMENT for i386 in another place. > You are arguing for (a) and I argue for (b). > > Use 1 less file but require a new header. > Use 1 more file but use existing headers. > > I argue that it's simpler and easier for developers if we avoid additional headers > and use existing headers. > Because your suggestion defines MALLOC_ALIGNMENT for i386 in more than one place, I argue it is more complex and more error prone than a single MALLOC_ALIGNMENT definition for i386. With my approach, it is fixed once for all.
On 06/30/2017 09:35 AM, H.J. Lu wrote: > On Fri, Jun 30, 2017 at 6:05 AM, Carlos O'Donell <carlos@redhat.com> wrote: >> On 06/30/2017 07:54 AM, H.J. Lu wrote: >>> On Thu, Jun 29, 2017 at 1:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>> Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT? >>>> >>>> 1. generic i386. >>>> 2. nptl i386. >>>> 3. hurd i386. >>>> >>>> That is very weird. >> >> I hope my explanation below makes the difference clearer. >> >>> I built and checked them for i686 with GCC 7. The biggest challenge is to >>> support GCC 7, not the header file itself. >> >> We have an existing malloc-machine.h for this purpose. >> >> The design question is: >> >> (a) Should we add a new file malloc-alignment.h to specify a new parameter. >> - Adds new file malloc-alignment.h. >> - 2 new files. > > And define MALLOC_ALIGNMENT for i386 in one place. > >> >> (b) Should we use existing malloc-machine.h to include the new parameter? >> - Extends existing malloc-machine.h to new sysdep dirs. >> - 3 new files. > > And define MALLOC_ALIGNMENT for i386 in 3 difference places. > If we need to support a new OS or threading library, we need to > define MALLOC_ALIGNMENT for i386 in another place. > >> You are arguing for (a) and I argue for (b). >> >> Use 1 less file but require a new header. >> Use 1 more file but use existing headers. >> >> I argue that it's simpler and easier for developers if we avoid additional headers >> and use existing headers. >> > > Because your suggestion defines MALLOC_ALIGNMENT for i386 in > more than one place, I argue it is more complex and more error > prone than a single MALLOC_ALIGNMENT definition for i386. With > my approach, it is fixed once for all. Can't they all equally just include 'sysdeps/i386/malloc-machine.h'? /* Include the base i386 definitions. */ #include <sysdeps/i386/malloc-machine.h> So you have one definition in one place? In sysdeps/nptl/malloc-machine.h we already have: #include <sysdeps/generic/malloc-machine.h> So we already use a layered approach to avoid multiple definitions.
On Fri, 30 Jun 2017, Carlos O'Donell wrote: > (a) Should we add a new file malloc-alignment.h to specify a new parameter. > - Adds new file malloc-alignment.h. > - 2 new files. My preference is to add headers like this and so reduce the need for #ifndef (that is, have a generic malloc-alignment that includes the definition that's currently guarded with #ifndef).
diff --git a/sysdeps/generic/malloc-alignment.h b/sysdeps/generic/malloc-alignment.h new file mode 100644 index 0000000..193e827 --- /dev/null +++ b/sysdeps/generic/malloc-alignment.h @@ -0,0 +1,24 @@ +/* Define MALLOC_ALIGNMENT for malloc. Generic version. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#ifndef _GENERIC_MALLOC_ALIGNMENT_H +#define _GENERIC_MALLOC_ALIGNMENT_H + +/* Use the default MALLOC_ALIGNMENT. */ + +#endif /* !defined(_GENERIC_MALLOC_ALIGNMENT_H) */ diff --git a/sysdeps/generic/malloc-machine.h b/sysdeps/generic/malloc-machine.h index 21aa9fc..4491b90 100644 --- a/sysdeps/generic/malloc-machine.h +++ b/sysdeps/generic/malloc-machine.h @@ -21,6 +21,7 @@ #define _GENERIC_MALLOC_MACHINE_H #include <atomic.h> +#include <malloc-alignment.h> #ifndef atomic_full_barrier # define atomic_full_barrier() __asm ("" ::: "memory") diff --git a/sysdeps/i386/malloc-alignment.h b/sysdeps/i386/malloc-alignment.h new file mode 100644 index 0000000..f72f7a8 --- /dev/null +++ b/sysdeps/i386/malloc-alignment.h @@ -0,0 +1,24 @@ +/* Define MALLOC_ALIGNMENT for malloc. i386 version. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#ifndef _I386_MALLOC_ALIGNMENT_H +#define _I386_MALLOC_ALIGNMENT_H + +#define MALLOC_ALIGNMENT 16 + +#endif /* !defined(_I386_MALLOC_ALIGNMENT_H) */