[Bug c/105676] New: Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))`

[Bug c/105676] New: Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))`

[sagebar at web dot de]

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

            Bug ID: 105676
           Summary: Bogus `-Wsuggest-attribute=pure` on function marked
                    `__attribute__((const))`
           Product: gcc
           Version: 12.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sagebar at web dot de
  Target Milestone: ---

When compiling with `-Wsuggest-attribute=pure`, gcc warns about missing
`__attribute__((pure))` on functions declared as `__attribute__((const))`.

It is common knowledge that any function marked as `__attribute__((const))`
also implicitly has the behavior of a function marked as
`__attribute__((pure))` (const implies pure).

Therefor, it stands to reason that such a warning is bogus (also: earlier
version of gcc didn't emit a warning in this case; know: gcc-9 already
supported these warnings, but didn't emit `-Wsuggest-attribute=pure` on a
`__attribute__((const))` function).

Example (`gcc -Wsuggest-attribute=pure -c -O2 input.c`):

```
__attribute__((const))
extern int do_expensive_calculation(void);

__attribute__((const))
int getval(void) {
        static int cache = -1;
        if (cache == -1)
                cache = do_expensive_calculation();
        return cache;
}
```

>test.c: In function 'getval':
>test.c:5:5: warning: function might be candidate for attribute 'pure' >[-Wsuggest-attribute=pure]
>    5 | int getval(void) {
>      |     ^~~~~~

When trying to declare as both pure+const:
>test.c:5:1: warning: ignoring attribute 'const' because it conflicts with attribute 'pure' [-Wattributes]
>    5 | int getval(void) {
>      | ^~~


==== Explaination of why using `__attribute__((const))` is valid here ====

I see why gcc might think that `getval()` is *only* `pure`, but there is
nothing wrong with the `__attribute__((const))` annotation since we don't "read
global memory" (emphasis on the "global"), and thus don't depend on the global
state (also: what counts as "global" vs. "non-global" isn't something that can
be quantified. - It depends on the application and how memory is used).

As such, the use of `__attribute__((const))` is very much valid (and gcc might
even consider suggesting `__attribute__((const))` instead of
`__attribute__((pure))`, since because `cache` is scoped-static, it not being
used outside of `getval()` can actually be proven, though that isn't what this
bug report is about...). However, so-long as it believes that the function is
pure, there is no reason to ever emit a about that fact so-long as `getval()`
remains annotated as `__attribute__((const))`.

==== End of Explaination ====

[Bug ipa/105676] [12/13 Regression] Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))`

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-05-20
             Status|UNCONFIRMED                 |NEW
      Known to fail|                            |12.1.0
      Known to work|                            |11.2.0, 11.3.0
     Ever confirmed|0                           |1
   Target Milestone|---                         |12.2
            Summary|Bogus                       |[12/13 Regression] Bogus
                   |`-Wsuggest-attribute=pure`  |`-Wsuggest-attribute=pure`
                   |on function marked          |on function marked
                   |`__attribute__((const))`    |`__attribute__((const))`

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.

[Bug ipa/105676] [12/13 Regression] Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))`

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug ipa/105676] [12/13 Regression] Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))` since r12-5437-g09a4ffb72aa2f513

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13 Regression] Bogus    |[12/13 Regression] Bogus
                   |`-Wsuggest-attribute=pure`  |`-Wsuggest-attribute=pure`
                   |on function marked          |on function marked
                   |`__attribute__((const))`    |`__attribute__((const))`
                   |                            |since
                   |                            |r12-5437-g09a4ffb72aa2f513
                 CC|                            |hubicka at gcc dot gnu.org
           Keywords|needs-bisection             |

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-5437-g09a4ffb72aa2f513.

[Bug ipa/105676] [12/13 Regression] Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))` since r12-5437-g09a4ffb72aa2f513

[hubicka at gcc dot gnu.org]

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

--- Comment #3 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Such code is not that obviously safe.  It is possible that getval will get
inlined to some calls and not other within single function.  In that case the
calling function will read and modify cache variable and will assume that these
are not changed across uninlined calls to getval.

After inlinning GCC might see something like:

        if (cache == -1)
                cache = do_expensive_calculation();
        val = getval ();

So now the question is whether one can convince gcc to duplicate and re-order
the code into something like:

        if (cache == -1)
                val = getval ();
                cache = do_expensive_calculation();
        else
                val = getval ();

which technically is valid transformation given what GCC is given and would
result in duplicatd expensive calculation.  I am not sure how to make this
happen.

tracer pass may be convinced to do the duplication:

        if (cache == -1)
                cache = do_expensive_calculation();
                val = getval ();
        else
                val = getval ();

but I don't think we currently have transform that will reorder val = getval()
the way I need.  This does not promise we won't have in future.  For example it
would be good optimizations to swap calls here:
__attribute__ ((const)) test(int);
__attribute__ ((const)) test2(int);

int ret (int a, int e)
{
        int c = test2 (2);
        int b = test (1);
        return c + b/e;
}

since division is a long operation and would benefit from b being ready earlier
than c.  It is kind of defect of our scheduler that we don't seem to do that.

I will look at the confused warning.  We should not suggest pure when function
is already const and I tought we already check for that.

I think we may want extra attribute for such caching functions.  Internally GCC
should make difference between "I can remove repeated calls to this function"
and "this function have no side effects".

[Bug ipa/105676] [12/13 Regression] Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))` since r12-5437-g09a4ffb72aa2f513

[mibjst9as at mozmail dot com]

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

paulober <mibjst9as at mozmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mibjst9as at mozmail dot com

--- Comment #5 from paulober <mibjst9as at mozmail dot com> ---
I have also problems with the newest version of GCC [12.2.0] suggesting pure
when we have const, suggesting const when we have pure (for __atribute__
((...))). 
You can find the code i'm reffering to tracted in this PR:
https://github.com/pi-hole/FTL/pull/1428

[Bug ipa/105676] [12/13 Regression] Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))` since r12-5437-g09a4ffb72aa2f513

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
It's ipa-modref detecting getval as pure and calling ipa_make_function_pure
which emits the diagnostic.  I have a patch excluding functions already const
from this.

[Bug ipa/105676] [12/13 Regression] Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))` since r12-5437-g09a4ffb72aa2f513

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:45e09c2eb9c2bdd34ef777e06ddc9908dd0664f9

commit r13-4535-g45e09c2eb9c2bdd34ef777e06ddc9908dd0664f9
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Dec 7 10:26:01 2022 +0100

    ipa/105676 - pure attribute suggestion for const function

    When a function is declared const (even though it technically
    accesses memory), ipa-modref discovering pureness shouldn't end
    up suggesting that attribute.  The following thus exempts
    'const' functions from ipa_make_function_pure handling.

            PR ipa/105676
            * ipa-pure-const.cc (ipa_make_function_pure): Skip also
            for functions already being const.

            * gcc.dg/pr105676.c: New testcase.

[Bug ipa/105676] [12 Regression] Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))` since r12-5437-g09a4ffb72aa2f513

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13 Regression] Bogus    |[12 Regression] Bogus
                   |`-Wsuggest-attribute=pure`  |`-Wsuggest-attribute=pure`
                   |on function marked          |on function marked
                   |`__attribute__((const))`    |`__attribute__((const))`
                   |since                       |since
                   |r12-5437-g09a4ffb72aa2f513  |r12-5437-g09a4ffb72aa2f513
      Known to work|                            |13.0
      Known to fail|12.1.0                      |12.2.0

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.

[Bug ipa/105676] [12 Regression] Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))` since r12-5437-g09a4ffb72aa2f513

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:96785885ceed34638d4b58e88cba6e6e8368c0e3

commit r12-9407-g96785885ceed34638d4b58e88cba6e6e8368c0e3
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Dec 7 10:26:01 2022 +0100

    ipa/105676 - pure attribute suggestion for const function

    When a function is declared const (even though it technically
    accesses memory), ipa-modref discovering pureness shouldn't end
    up suggesting that attribute.  The following thus exempts
    'const' functions from ipa_make_function_pure handling.

            PR ipa/105676
            * ipa-pure-const.cc (ipa_make_function_pure): Skip also
            for functions already being const.

            * gcc.dg/pr105676.c: New testcase.

    (cherry picked from commit 45e09c2eb9c2bdd34ef777e06ddc9908dd0664f9)

[Bug ipa/105676] [12 Regression] Bogus `-Wsuggest-attribute=pure` on function marked `__attribute__((const))` since r12-5437-g09a4ffb72aa2f513

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
      Known to work|                            |12.2.1
             Status|ASSIGNED                    |RESOLVED

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.