[Bug target/58065] New: ARM MALLOC_ABI_ALIGNMENT is wrong

[Bug target/58065] New: ARM MALLOC_ABI_ALIGNMENT is wrong

[bernd.edlinger at hotmail dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58065

            Bug ID: 58065
           Summary: ARM MALLOC_ABI_ALIGNMENT is wrong
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: bernd.edlinger at hotmail dot de
            Target: arm*-*-*

the ARM target architecture does not define the MALLOC_ABI_ALIGNMENT,
therefore the default is used as BITS_PER_WORD, 32 in this case.

This produces sometimes suboptimal code, because the front-end
assumes that the function malloc() returns only word-aligned pointers,
which is likely wrong. I have not found any specific requirements
on the malloc alignment in the AAPCS document at
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042e/IHI0042E_aapcs.pdf
but I believe that the intention is to align everything including stack
pointers to 8 bytes.

Therefore I would suggest the attached patch which defines
MALLOC_ABI_ALIGNMENT as BIGGEST_ALIGNMENT, which is 64 bits.

As a proof that this has indeed some subtle influence on the generated code
I attach a test case. The function foo is called by bar, and bar uses
malloc to allocate the memory, with compiler options "-O3 -g0 -mfpu=neon
-mfloat-abi=softfp" the function foo is inlined into bar,
but the inlined version does not use vstr instructions any more, because
the front-end does assume that malloc returns 4 byte aligned memory.

If that was really true, foo must fail, if it is called without inlining.
Therefore this code is just clumsy and less optimal than it could be.

[Bug target/58065] ARM MALLOC_ABI_ALIGNMENT is wrong

[bernd.edlinger at hotmail dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58065

--- Comment #1 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Created attachment 30598
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30598&action=edit
test case

[Bug target/58065] ARM MALLOC_ABI_ALIGNMENT is wrong

[bernd.edlinger at hotmail dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58065

--- Comment #2 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Created attachment 30599
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30599&action=edit
compiler output without this patch

[Bug target/58065] ARM MALLOC_ABI_ALIGNMENT is wrong

[bernd.edlinger at hotmail dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58065

--- Comment #3 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Created attachment 30600
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30600&action=edit
correct compiler output with patch

[Bug target/58065] ARM MALLOC_ABI_ALIGNMENT is wrong

[bernd.edlinger at hotmail dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58065

--- Comment #4 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Created attachment 30601
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30601&action=edit
Proposed patch

[Bug target/58065] ARM MALLOC_ABI_ALIGNMENT is wrong

[david.abdurachmanov at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58065

David Abdurachmanov <david.abdurachmanov at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |david.abdurachmanov at gmail dot c
                   |                            |om

--- Comment #5 from David Abdurachmanov <david.abdurachmanov at gmail dot com> ---
malloc() [glibc implementation] default alignment is sizeof(long double) or 2 *
sizeof(size_t) if I remember correctly, which is 8 bytes for ARMv7. I think,
based on C and C++ standard you have to make sure that alignment is good for
whatever primitive type, which means alignment size being the size of the
biggest primitive type (long double).

Reference bug ticket(9 years old):
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15795

Quote from C standard (identical or similar exist in C++):

The pointer returned if the allocation succeeds is suitably aligned so that
it may be assigned to a pointer to any type of object and then used to
access such an object or an array of such objects in the space allocated
(until the space is explicitly deallocated).

[Bug target/58065] ARM MALLOC_ABI_ALIGNMENT is wrong

[ramana at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58065

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ramana at gcc dot gnu.org

--- Comment #6 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
(In reply to Bernd Edlinger from comment #4)
> Created attachment 30601 [details]
> Proposed patch

If you want to propose a patch please post to the mailing list. 

Ramana

[Bug target/58065] ARM MALLOC_ABI_ALIGNMENT is wrong

[bernd.edlinger at hotmail dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58065

--- Comment #7 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Patch was posted here: http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00350.html

[Bug target/58065] ARM MALLOC_ABI_ALIGNMENT is wrong

[clyon at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58065

--- Comment #9 from clyon at gcc dot gnu.org ---
Author: clyon
Date: Thu Sep  5 12:27:56 2013
New Revision: 202276

URL: http://gcc.gnu.org/viewcvs?rev=202276&root=gcc&view=rev
Log:
2013-09-05  Christophe Lyon  <christophe.lyon@linaro.org>

    Backport from trunk r201589.
    2013-08-08  Bernd Edlinger  <bernd.edlinger@hotmail.de>

    PR target/58065
    * config/arm/arm.h (MALLOC_ABI_ALIGNMENT): Define.


Modified:
    branches/linaro/gcc-4_8-branch/   (props changed)
    branches/linaro/gcc-4_8-branch/gcc/ChangeLog.linaro
    branches/linaro/gcc-4_8-branch/gcc/config/arm/arm.h

Propchange: branches/linaro/gcc-4_8-branch/
            ('svn:mergeinfo' modified)