This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] target/58065 ARM MALLOC_ABI_ALIGNMENT is wrong

On 08/07/13 08:10, Bernd Edlinger wrote:

in the discussion about the PR middle-end/57748 it became obvious that the ARM target architecture should define a value for MALLOC_ABI_ALIGNMENT, because otherwise the default is simply word aligned, which causes sub-optimal code, at least for the neon fpu.

This simple patch fixes PR target/58065 by defining the MALLOC_ABI_ALIGNMENT as BIGGEST_ALIGNMNET.

As a proof that this has indeed some subtle influence on the generated code
I created a new 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.

You don't mention whether this patch was regression tested. All patches must be regression tested. I've applied only the changes to the source after testing it overnight with a couple of my other patches. Read further on about why this testcase is not suitable and how you could make this better for next time.

For next time please read the contribution web page with respect to testing here

Bernd Edlinger 		 	   		

Now moving on to the patch itself -

The testcase doesn't work because the vstr's that come out aren't because the alignment information doesn't match up rather than it being because the compiler has decided to use immediate offset addressing on storing a vector. Once you do that for a 128 bit vector there is no option but for the compiler to put out 2 vstr instructions which happens even after your patch for me.

+++ gcc/testsuite/	2013-08-03 00:08:22.000000000 +0200
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -g0 -mfpu=neon -mfloat-abi=softfp" } */

For next time please use dg-add-options arm_neon for the -mfpu=neon -mfloat-abi=softfp bits. You also shouldn't need -g0 here.

Instead I'd scan an intermedate rtl dump for alignment information rather than anything else. I haven't yet looked at the output but looking for A64 in an RTL dump instead of A32 might help in this particular case.

Also reading will be useful with respect to creating new testcases.

+/* { dg-final { scan-assembler-not "\[^v]str" } } */
+#include <stdlib.h>
+typedef long long V
+  __attribute__ ((vector_size (2 * sizeof (long long)), may_alias));
+struct T { V a; V b[1]; };
+foo (struct T *p)
+  V a = { 3, 4 };
+  p->b[0] = a;
+struct T *
+bar ()
+  struct T *t = (struct T *) malloc (sizeof(*t));
+  foo (t);
+  return t;

Please post the changelog inline in the future.

> 2013-08-07  Bernd Edlinger  <>
> 	PR target/58065
> 	* gcc/config/arm/arm.h: Define MALLOC_ABI_ALIGNMENT.
> 	* gcc/testsuite/ New testcase.

Also for next time there should be a changelog entry for each ChangeLog file you need to touch in this case with the file names in the changelog entry rooted at the file in which the ChangeLog file is present. In this particular case you need one for the source base and one for the testsuite and drop the gcc/ and gcc/testsuite prefixes to the file names below. Also notice how I've changed the Changelog to actually refer to MALLOC_ABI_ALIGNMENT rather than stating what's changed for arm.h.

Therefore this should read

2013-08-08  Bernd Edlinger  <>

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

2013-08-08  Bernd Edlinger  <>

	PR target/58065
 	* New testcase.

This makes it easier for the person applying the patch to copy paste the Changelog into the relevant file. Thanks :)

And finally thank you for submitting the patch, if you intend on contributing to GCC in the medium to longer term, it would be worth getting the process for obtaining a copyright assignment sorted if you haven't done so already.

If you send a request for a copyright assignment form to, a maintainer who has access to the relevant forms will forward this on to you.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]