public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Sebor <msebor@gmail.com>
To: Jeff Law <law@redhat.com>, GCC Mailing List <gcc@gcc.gnu.org>
Subject: Re: question about attribute aligned for functions
Date: Tue, 27 Nov 2018 22:43:00 -0000	[thread overview]
Message-ID: <d0c5a486-6031-34e2-db06-1e6464270c15@gmail.com> (raw)
In-Reply-To: <7e984f57-1ccb-138e-fb53-51c771b3227d@gmail.com>

On 11/27/18 11:57 AM, Martin Sebor wrote:
> On 11/26/18 3:37 PM, Jeff Law wrote:
>> On 11/23/18 12:31 PM, Martin Sebor wrote:
>>> GCC currently accepts the declaration of f0 below but ignores
>>> the attribute.  On aarch64 (and I presume on other targets with
>>> a default function alignment greater than 1), GCC rejects f1
>>> with an error, even though it accepts -falign-functions=1
>>> without as much as a warning.
>>>
>>> Clang, on the other hand, rejects f0 with a hard error because
>>> the alignment is not a power of two, but accepts f1 and appears
>>> to honor the attribute.  It also accepts -falign-functions=1.
>>>
>>> I think diagnosing f0 with a warning is helpful because an explicit
>>> zero alignment is most likely a mistake (especially when it comes
>>> from a macro or some computation).
>>>
>>> But I don't see a good reason to reject a program that specifies
>>> a smaller alignment for a function when the default (or minimum)
>>> alignment is greater.  A smaller alignment is trivially satisfied
>>> by a greater alignment so either accepting it or dropping it seems
>>> preferable to failing with an error (either could be with or without
>>> a warning).
>>>
>>>    __attribute__ ((aligned (0))) void f0 (void);   // accepted, ignored
>>>    __attribute__ ((aligned (1))) void f1 (void);   // aarch64 error
>>>    __attribute__ ((aligned (4))) void f4 (void);   // okay
>>>
>>> Does anyone know why GCC rejects the program, or can anyone think
>>> of a reason why GCC should not behave as suggested above?
>> Note we have targets that support single byte opcodes and do not have
>> any requirements for functions starting on any boundary.  mn103 comes to
>> mind.
>>
>> However, the attribute can't be used to decrease a function's alignment,
>> so values of 0 or 1 are in practice totally uninteresting and one could
>> make an argument to warn for them.
> 
> The attribute does reduce the default alignment at least on some
> targets.  For instance, on x86_64 it lowers it from the default
> 16 to as little as 2, but it silently ignores 1.
> 
> At the same time, the following passes on x86_64:
> 
>    __attribute__ ((aligned (1))) void f1 (void) { }
>    _Static_assert (__alignof__ (f1) == 1);   // wrong alignof result
> 
>    __attribute__ ((aligned)) void f0 (void) { }
>    _Static_assert (__alignof__ (f0) == 16);
> 
>    __attribute__ ((aligned (2))) void f2 (void) { }
>    _Static_assert (__alignof__ (f2) == 2);
> 
> but the assembly shows no .align directive for f1, .align 16 for
> f0, and .align 2 for f2, and the object file shows f1 first with
> padding up to 16-bytes, followed by f0 padded to a 2 byte
> boundary, followed by f2.  The picture stays the same when f1()
> is declared without the attribute (i.e., it's 16-byte aligned
> by default).  So __alignof__ reports the wrong alignment for
> the f1 declared aligned (1).

Actually, after some thought and experimenting I don't believe
the alignment __alignof__ reports is wrong.  It's the best it
can do.  The actual alignment of the function in the object file
could be greater (e.g., depending on what other functions are
before or after it), and that should be fine.

GCC should also be able to optimize the size of the emitted code
by rearranging functions based on both their size and alignment
requirements, perhaps by increasing both for some functions if
that reduces the size overall.  This optimization can only be
done after __alignof__ expressions have been evaluated.

So with that, I think the argument to attribute aligned must
inevitably be viewed as the lower bound on a function's (or even
variable's) alignment, and specifying a value that's smaller than
what's ultimately provided should be accepted without a warning.
I think that should hold for strictly aligned targets like sparc
with a minimum alignment greater than 1 as well as for relaxed
ones like i386 with a minimum alignment of 1.

I will make this change to the aligned attribute handler to avoid
the failures in the builtin-has-attribute-*.c tests on strictly
aligned targets.

>> Whether or not to warn in general if the alignment attribute is smaller
>> than the default may be subject to debate.  I guess it depends on the
>> general intent that we'd find in real world codes.
> 
> I would expect real world code to care about achieving at least
> the specified alignment.
> 
> A less restrictive alignment requirement is satisfied by providing
> a more restrictive one, and (the above notwithstanding) the manual
> documents that
> 
>    You cannot use this attribute to decrease the alignment of
>    a function, only to increase it.
> 
> So I wouldn't expect real code to be relying on decreasing
> the alignment.
> 
> That said, since GCC does make it possible to decrease the default
> alignment of functions, I can think of no reason for it not to
> continue when it's possible.  I.e., on targets with underaligned
> instruction reads to be honor requests for underaligned functions.
> On strictly aligned targets I think the safe thing to do is to
> quietly ignore requests for smaller alignments by default, and
> perhaps provide a new option to request a warning (with the warning
> being disabled by default).
> 
> Do you see a problem with this approach?
> 
> Martin

  reply	other threads:[~2018-11-27 22:35 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-23 22:40 Martin Sebor
2018-11-26 23:37 ` Jeff Law
2018-11-27 22:35   ` Martin Sebor
2018-11-27 22:43     ` Martin Sebor [this message]
2018-11-28 13:41     ` Florian Weimer
2018-11-28 17:01       ` Martin Sebor
2018-11-29  9:45     ` Jeff Law
2018-11-29 18:29       ` Martin Sebor
2018-12-01  0:12         ` Jeff Law

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d0c5a486-6031-34e2-db06-1e6464270c15@gmail.com \
    --to=msebor@gmail.com \
    --cc=gcc@gcc.gnu.org \
    --cc=law@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).