diff mbox

PATCH: PR target/61296: Excessive alignment in ix86_data_alignment

Message ID 20141217144901.GA24101@gmail.com
State New
Headers show

Commit Message

H.J. Lu Dec. 17, 2014, 2:49 p.m. UTC
On Wed, Dec 17, 2014 at 03:31:50PM +0100, Uros Bizjak wrote:
> On Wed, Dec 17, 2014 at 2:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> 
> > Add -malign-data={abi|compat,cachineline} to control how GCC aligns
> > variables.  "compat" uses increased alignment value compatible with
> > GCC 4.8 and earlier, "abi" uses alignment value as specified by the
> > psABI, and "cacheline" with increased alignment value to match the
> > cache line size.  "compat" is the default.
> >
> > gcc/
> >
> >         PR target/61296
> >         * config/i386/i386-opts.h (ix86_align_data): New enum.
> >         * config/i386/i386.c (ix86_data_alignment): Return the ABI
> >         alignment value for -malign-data=abi, the cachine line size
> >         for -malign-data=cachineline and the older GCC compatible
> >         alignment value for for -malign-data=compat.
> >         * config/i386/i386.opt (malign-data=): New.
> >         * doc/invoke.texi: Document -malign-data=.
> 
> Please also mention new user-visible options in GCC changes document.
> 

Here is a patch.  OK to install?

> (Oh, just spotted a trivial typo in the ChangeLog entry: cachineline
> -> cacheline).

Fixed.

Thanks.


H.J.
---

Comments

Gerald Pfeifer Dec. 26, 2014, 4:56 a.m. UTC | #1
On Wednesday 2014-12-17 06:49, H.J. Lu wrote:
> Index: gcc-5/changes.html
> ===================================================================
> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-5/changes.html,v
> retrieving revision 1.52
> diff -u -p -r1.52 changes.html
> --- gcc-5/changes.html	15 Dec 2014 19:55:08 -0000	1.52
> +++ gcc-5/changes.html	17 Dec 2014 14:48:21 -0000
> @@ -444,6 +444,13 @@ void operator delete[] (void *, std::siz
>  	place of the __fentry__ or mcount call, so that a call per function
>  	can be later patched in. This can be used for low overhead tracing or
>  	hot code patching.</li>
> +	<li> The new <code>-malign-data=</code> option to control how
> +	GCC aligns variables.

Let's make this "...option controls how..."

This is fine with this change and considering the genuine question 
below.

>  <code>-malign-data=compat</code> uses
> +	increased alignment value compatible with GCC 4.8 and earlier,
> +	<code>-malign-data=abi</code> uses alignment value as specified by
> +	the psABI, and <code>-malign-data=cacheline</code> uses increased
> +	alignment value to match the cache line size.
> +	<code>-malign-data=compat</code> is the default.</li>

Here, and in the .texi documentation, would it be appropriate to
just say "alignment" instead of "alignment value" throughout, or
is there particular reason to say the latter?

Gerald
H.J. Lu Dec. 26, 2014, 3:49 p.m. UTC | #2
On Thu, Dec 25, 2014 at 8:56 PM, Gerald Pfeifer <gerald@pfeifer.com> wrote:
> On Wednesday 2014-12-17 06:49, H.J. Lu wrote:
>> Index: gcc-5/changes.html
>> ===================================================================
>> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-5/changes.html,v
>> retrieving revision 1.52
>> diff -u -p -r1.52 changes.html
>> --- gcc-5/changes.html        15 Dec 2014 19:55:08 -0000      1.52
>> +++ gcc-5/changes.html        17 Dec 2014 14:48:21 -0000
>> @@ -444,6 +444,13 @@ void operator delete[] (void *, std::siz
>>       place of the __fentry__ or mcount call, so that a call per function
>>       can be later patched in. This can be used for low overhead tracing or
>>       hot code patching.</li>
>> +     <li> The new <code>-malign-data=</code> option to control how
>> +     GCC aligns variables.
>
> Let's make this "...option controls how..."

Done.

> This is fine with this change and considering the genuine question
> below.
>
>>  <code>-malign-data=compat</code> uses
>> +     increased alignment value compatible with GCC 4.8 and earlier,
>> +     <code>-malign-data=abi</code> uses alignment value as specified by
>> +     the psABI, and <code>-malign-data=cacheline</code> uses increased
>> +     alignment value to match the cache line size.
>> +     <code>-malign-data=compat</code> is the default.</li>
>
> Here, and in the .texi documentation, would it be appropriate to
> just say "alignment" instead of "alignment value" throughout, or
> is there particular reason to say the latter?

I don't have a strong opinion on it.  Please feel free to improve it.

Thanks.
diff mbox

Patch

Index: gcc-5/changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-5/changes.html,v
retrieving revision 1.52
diff -u -p -r1.52 changes.html
--- gcc-5/changes.html	15 Dec 2014 19:55:08 -0000	1.52
+++ gcc-5/changes.html	17 Dec 2014 14:48:21 -0000
@@ -444,6 +444,13 @@  void operator delete[] (void *, std::siz
 	place of the __fentry__ or mcount call, so that a call per function
 	can be later patched in. This can be used for low overhead tracing or
 	hot code patching.</li>
+	<li> The new <code>-malign-data=</code> option to control how
+	GCC aligns variables.  <code>-malign-data=compat</code> uses
+	increased alignment value compatible with GCC 4.8 and earlier,
+	<code>-malign-data=abi</code> uses alignment value as specified by
+	the psABI, and <code>-malign-data=cacheline</code> uses increased
+	alignment value to match the cache line size.
+	<code>-malign-data=compat</code> is the default.</li>
   </ul>
 
 <h3 id="sh">SH</h3>