Bug 96247 - -falign-functions=0 works wrongly
Summary: -falign-functions=0 works wrongly
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 11.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: documentation
Depends on:
Blocks:
 
Reported: 2020-07-20 06:33 UTC by hujp
Modified: 2020-07-27 17:51 UTC (History)
2 users (show)

See Also:
Host:
Target: aarch64, x86_64-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-07-20 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description hujp 2020-07-20 06:33:10 UTC
When reading the description of -falign-functions=n in doc/invoke.texi
or gcc man page, I did the following test and found a problem.

---
> # sed -n '11333,11334p' invoke.texi
> If @var{n} is not specified or is zero, use a machine-dependent default.
> The maximum allowed @var{n} option value is 65536.
> 
> $ cat hello.c
> int foo() {
>         int s = 0;
> 
>         for (int i=0; i < 10; i++) {
>                 s += i;
>         }
> 
>         return s;
> }
> 
> $ /home/extra_mnt/cross_build_gcc/bin/aarch64-linux-gcc -falign-functions -S hello.c; mv hello.s hello.s.align
> $ /home/extra_mnt/cross_build_gcc/bin/aarch64-linux-gcc -falign-functions=0 -S hello.c; mv hello.s hello.s.0
> $ diff -u hello.s.align hello.s.0
> --- hello.s.align       2020-07-20 10:57:30.660038984 +0800
> +++ hello.s.0   2020-07-20 10:57:33.689038984 +0800
> @@ -2,7 +2,6 @@
>         .file   "hello.c"
>         .text
>         .align  2
> -       .p2align 4,,11
>         .global foo
>         .type   foo, %function
>  foo:
>
> 
> $ /home/extra_mnt/cross_build_gcc/bin/aarch64-linux-gcc --version
> aarch64-linux-gcc (GCC) 11.0.0 20200519 (experimental)
> Copyright (C) 2020 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
---

I think the compiler's behavior is inconsistent with the doc, the
assembly code should be no defference when -falign-functions option
or -falign-functions=0 option is specified.
I found that the parameter which the target sources like
gcc/config/aarch64/aarch64.c used was the command line argument
'0', not the machine-dependent parameters which should overide the 
'0'. So I have submit a patch to gcc-patches list, but still have no 
comments.
Comment 1 Richard Biener 2020-07-20 07:18:02 UTC
Respective

--
If @var{n} is not specified or is zero, use a machine-dependent default.
The maximum allowed @var{n} option value is 65536.
--

not sure if that's sensible or if instead zero should be rejected.  Note
x86_64 behaves the same so it's probably better to adjust the documentation
to reflect the implementation behavior.
Comment 2 hujp 2020-07-20 08:41:43 UTC
(In reply to Richard Biener from comment #1)
> not sure if that's sensible or if instead zero should be rejected.  Note
> x86_64 behaves the same so it's probably better to adjust the documentation
> to reflect the implementation behavior.
Thanks, Richard.

Yes, I confirmed that x86_64 have the same problem in the source level.

From the point of view of the function of the option, it is meaningless to align with 0, so for -falign-function=0, 
  - either has a special meaning, that is, use the default value (that is, the document now says), 
  - or should be rejected. 
I prefer the latter, if that don't need to modify the target code (for now, I haven't confirmed the relevant code). It means that both the code and the documentation need to be modified a bit.

On the other hand, just adjusting the documentation to reflect the implementation behavior will cause -falign-functions=0 and -falign-functions=1 to have the same meaning, which is a bit confusing from the user's perspective.

What do you think?
Comment 3 Martin Sebor 2020-07-21 15:37:56 UTC
There is an interplay between -falign-functions and the aligned function attribute so changes to one (either the implementation or the documentation) should be reviewed for an impact on the other and vice versa.

Attribute aligned is documented to require "an integer constant power of 2," with other values, including zero, being ignored with a warning.  Keeping the option in sync with the attribute (and warning on the same set on invalid arguments) seems like a good approach to me.
Comment 4 CVS Commits 2020-07-27 17:22:54 UTC
The master branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:d60758c74af849539f5dd71a8456eea954c0f674

commit r11-2368-gd60758c74af849539f5dd71a8456eea954c0f674
Author: Hu Jiangping <hujiangping@cn.fujitsu.com>
Date:   Mon Jul 27 18:22:23 2020 +0100

    driver: fix a problem with implementation of -falign-foo=0 [PR96247]
    
    This patch makes the -falign-foo=0 work as described in the
    documentation. Thanks for all the suggestions.
    
    Changelog:
    2020-07-27  Hu Jiangping  <hujiangping@cn.fujitsu.com>
    
            PR driver/96247
            * opts.c (check_alignment_argument): Set the -falign-Name
            on/off flag on and set the -falign-Name string value null,
            when the command-line specified argument is zero.
Comment 5 rsandifo@gcc.gnu.org 2020-07-27 17:51:11 UTC
Fixed on master.