[Bug target/96247] New: -falign-functions=0 works wrongly

[Bug target/96247] New: -falign-functions=0 works wrongly

[hujiangping at cn dot fujitsu.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96247

            Bug ID: 96247
           Summary: -falign-functions=0 works wrongly
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hujiangping at cn dot fujitsu.com
  Target Milestone: ---

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.

[Bug target/96247] -falign-functions=0 works wrongly

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96247

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unknown                     |11.0
           Keywords|                            |documentation
             Status|UNCONFIRMED                 |NEW
             Target|                            |aarch64, x86_64-*-*
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-07-20

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
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.

[Bug target/96247] -falign-functions=0 works wrongly

[hujiangping at cn dot fujitsu.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96247

--- Comment #2 from hujp <hujiangping at cn dot fujitsu.com> ---
(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?

[Bug target/96247] -falign-functions=0 works wrongly

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96247

Martin Sebor <msebor at gcc dot gnu.org> changed:

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

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
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.

[Bug target/96247] -falign-functions=0 works wrongly

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96247

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug target/96247] -falign-functions=0 works wrongly

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96247

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |rsandifo at gcc dot gnu.org
         Resolution|---                         |FIXED

--- Comment #5 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Fixed on master.