[Bug c++/107622] New: Missing optimization of switch-statement

[Bug c++/107622] New: Missing optimization of switch-statement

[klaus.doldinger64 at googlemail dot com]

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

            Bug ID: 107622
           Summary: Missing optimization of switch-statement
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: klaus.doldinger64 at googlemail dot com
  Target Milestone: ---

In the following example the default-case could not be reached. Therefore
introducing std::unreachable() should be useless. But the compiler produces
slightly better code with std::unreachable() as it removes one (unneccessary)
comparison (tested for x86 and avr targets).

volatile uint8_t o;

enum class State : uint8_t {A, B, C};

void g(const State s) {
    switch(s) {
    case State::A:
        o = 10;
        break;
    case State::B:
        o = 11;
        break;
    case State::C:
        o = 12;
        break;
    default:
//        std::unreachable(); // __builtin_unreachable();
        break;
    }
}

[Bug c++/107622] Missing optimization of switch-statement

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> In the following example the default-case 

Yes it can. You can pass a 0xf to that function and still have well defined
behavior.

[Bug c++/107622] Missing optimization of switch-statement

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
There is one part which GCC could optimize better that is the store.
There might be another bug about that too.

Note as I mentioned __builtin_unreachable is needed if truely the value coming
inside g is outside of [0,3] range.

[Bug c++/107622] Missing optimization of switch-statement

[klaus.doldinger64 at googlemail dot com]

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

--- Comment #3 from Wilhelm M <klaus.doldinger64 at googlemail dot com> ---
(In reply to Andrew Pinski from comment #1)
> > In the following example the default-case 
> 
> Yes it can. You can pass a 0xf to that function and still have well defined
> behavior.

Oh yes, thank you for the hint!

[Bug c++/107622] Missing optimization of switch-statement

[klaus.doldinger64 at googlemail dot com]

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

--- Comment #4 from Wilhelm M <klaus.doldinger64 at googlemail dot com> ---
In the following class the static data member is accessible via explicit
template instantiation from the outside. So the compiler cannot reason that the
value is in [0,2]. But this does not hold for the function g(): here mState2 is
definitely not accessible from outside. But here the std::unreachable() is
still neccessary. here the optimization is missing. 

struct FSM {
    enum class State : uint8_t {A, B, C};
    static void f() {
        switch(mState) {
        case State::A:
            o = 10;
            break;
        case State::B:
            o = 11;
            break;
        case State::C:
            o = 12;
            break;
        default:
    //        std::unreachable();
            break;
        }
    }
    static void g() {
        static State mState2{State::A}; // not accessible from outside
        switch(mState2) {
        case State::A:
            o = 10;
            break;
        case State::B:
            o = 11;
            break;
        case State::C:
            o = 12;
            break;
        default:
    //        std::unreachable();
            break;
        }
    }
private:
    inline static State mState{State::A}; // still modifyable via explicit
template instantiation
};

[Bug c++/107622] Missing optimization of switch-statement

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Did you try -fstrict-enums?

[Bug c++/107622] Missing optimization of switch-statement

[klaus.doldinger64 at googlemail dot com]

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

--- Comment #6 from Wilhelm M <klaus.doldinger64 at googlemail dot com> ---
(In reply to Richard Biener from comment #5)
> Did you try -fstrict-enums?

Yes, that doesn't change anything.

[Bug c++/107622] Missing optimization of switch-statement

[glisse at gcc dot gnu.org]

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

--- Comment #7 from Marc Glisse <glisse at gcc dot gnu.org> ---
(Wilhelm, when you post testcases, please post the full file including the
#include lines)

(In reply to Richard Biener from comment #5)
> Did you try -fstrict-enums?

IIUC, even if optimizations using -fstrict-enums were implemented, they would
only help with the first testcase if the number of enum values was a power of
2. For {A,B,C}, -fstrict-enums still considers 3 a valid value.

I have long wanted an attribute to specify that a particular enum is only
allowed to take the values explicitly listed, though I cannot find a relevant
issue in bugzilla at the moment.


Comment #4 is an independent issue where gcc fails to notice that since the
static variable does not escape, it can be replaced with a local constant.

[Bug c++/107622] Missing optimization of switch-statement

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |marxin at gcc dot gnu.org
     Ever confirmed|0                           |1
                 CC|                            |marxin at gcc dot gnu.org
   Last reconfirmed|                            |2022-11-21
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
Will take a look.

[Bug c++/107622] Missing optimization of switch-statement

[marxin at gcc dot gnu.org]

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

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

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

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
So as was already said, -fstrict-enums is coarse-grained and can catch only a
situation like:

enum State {A, B, C};
...
void g(const State s) {
    switch(s) {
...
      case 257:
        o = 13;
        break;


orig.C:18:7: warning: case label value exceeds maximum value for type
[-Wswitch-outside-range]
   18 |       case 257:
      |       ^~~~

[Bug c++/107622] Missing optimization of switch-statement

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
If you use enum with fixed underlying type, then all the values of the
underlying type are valid, even with -fstrict-enums.  Otherwise
namespace std {
enum class byte : unsigned char {};
}
couldn't work properly.  Otherwise, with -fstrict-enums, enum { A, B, C } can
have
values 0, 1, 2, 3 and without -fstrict-enums all the values of the underlying
type.
See https://eel.is/c++draft/dcl.enum#8 for more details.
So, except for the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107622#c4 g
case where
we should have optimized it into constant I don't see any missed optimization
here.

[Bug c++/107622] Missing optimization of switch-statement

[klaus.doldinger64 at googlemail dot com]

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

--- Comment #11 from Wilhelm M <klaus.doldinger64 at googlemail dot com> ---
Without an underlying type but with -fshort-enums the underlying type should be
as small as possible. In this case this should be a uint8_t. But in this case
we get a 16-bit value for mState. This is a clear violation of -fshort-enums.

[Bug c++/107622] Missing optimization of switch-statement

[klaus.doldinger64 at googlemail dot com]

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

--- Comment #12 from Wilhelm M <klaus.doldinger64 at googlemail dot com> ---
(In reply to Wilhelm M from comment #11)
> Without an underlying type but with -fshort-enums the underlying type should
> be as small as possible. In this case this should be a uint8_t. But in this
> case we get a 16-bit value for mState. This is a clear violation of
> -fshort-enums.

That (my previous comment) was clear nonsense. enum class has always underlying
type int if not specified according to the standard.