[Bug target/97736] New: [9/10 Regression] switch codegen

[Bug target/97736] New: [9/10 Regression] switch codegen

[ncm at cantrip dot org]

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

            Bug ID: 97736
           Summary: [9/10 Regression] switch codegen
           Product: gcc
           Version: 9.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ncm at cantrip dot org
  Target Milestone: ---

In Gcc 8 and previous, the following code 

  bool is_vowel(char c) {
    switch (c)
    case'a':case'e':case'i':case'o':case'u':
      return true;
    return false;
  }

compiled with -O2 or better, for numerous x86-64 targets,
resolves to a bitwise flag check, e.g.

        lea     ecx, [rdi-97]
        xor     eax, eax
        cmp     cl, 20
        ja      .L1
        mov     eax, 1
        sal     rax, cl
        test    eax, 1065233
        setne   al
  .L1:
        ret

Starting in gcc-9, this optimization is not performed 
anymore at -O2 for many common targets (e.g. -march=skylake),
and we get

        sub     edi, 97
        cmp     dil, 20
        ja      .L2
        movzx   edi, dil
        jmp     [QWORD PTR .L4[0+rdi*8]]
  .L4:
        .quad   .L5
        .quad   .L2
        .quad   .L2
        .quad   .L2
        .quad   .L5
        .quad   .L2
        .quad   .L2
        .quad   .L2
        .quad   .L5
        .quad   .L2
        .quad   .L2
        .quad   .L2
        .quad   .L2
        .quad   .L2
        .quad   .L5
        .quad   .L2
        .quad   .L2
        .quad   .L2
        .quad   .L2
        .quad   .L2
        .quad   .L5
  .L2:
        mov     eax, 0
        ret
  .L5:
        mov     eax, 1
        ret

same as with -O0 or -O1.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |9.4
          Component|target                      |tree-optimization
            Summary|[9/10 Regression] switch    |[9/10/11 Regression] switch
                   |codegen                     |codegen
                 CC|                            |marxin at gcc dot gnu.org
           Keywords|                            |missed-optimization

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r9-1342-g789410e4824b60f423f7855d68f234e02f9300fb

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[marxin at gcc dot gnu.org]

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

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

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

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Mine then.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |WONTFIX

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
All right. So for the mentioned test-case we can expand both as jump table and
as a bit-test. Since the revision, we prefer the jump table.
You can disable that with -fno-jump-tables for the interesting function.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I disagree with that resolution, expanding it as a bit test is certainly better
and shorter too.
And it seems even switchconv pass says that it is preferrable to expand it as
bit-test, but during expansion it used to be expanded as bit test, but now it
just is not.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |ASSIGNED
         Resolution|WONTFIX                     |---

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
All right, let's prefer a bit-test over the jump table.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[ncm at cantrip dot org]

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

--- Comment #6 from ncm at cantrip dot org ---
The referenced patch seems to have also deleted a fair bit of explanatory
comment text, including a list of possible refinements for selected targets.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:5e303cdee1ff01e4b302ef2f913c0bdd84ab967e

commit r11-5049-g5e303cdee1ff01e4b302ef2f913c0bdd84ab967e
Author: Martin Liska <mliska@suse.cz>
Date:   Mon Nov 9 14:26:04 2020 +0100

    Prefer bit-test over the jump table.

    gcc/ChangeLog:

            PR tree-optimization/97736
            * tree-switch-conversion.c
(switch_decision_tree::analyze_switch_statement):
            Prefer bit tests.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/97736
            * gcc.dg/tree-ssa/switch-1.c: Prefer bit tests.
            * g++.dg/tree-ssa/pr97736.C: New test.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[marxin at gcc dot gnu.org]

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

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to ncm from comment #6)
> The referenced patch seems to have also deleted a fair bit of explanatory
> comment text, including a list of possible refinements for selected targets.

Sorry for that, I made an overhaul of the pass and I may miss porting of some
comments.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
Fixed on the master, not planning to backport.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[ncm at cantrip dot org]

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

--- Comment #10 from ncm at cantrip dot org ---
Don't understand, the compiler we are using (9) has the 
regression. It looks like a trivial backport.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[jakub at gcc dot gnu.org]

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
We usually don't backport optimization improvements, even if they fix
regressions, to release branches.  There are quite high risks involved.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[ncm at cantrip dot org]

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

--- Comment #12 from ncm at cantrip dot org ---
As it is, your probability of failure in 9 and 10 is exactly 1.0.

[Bug tree-optimization/97736] [9/10/11 Regression] switch codegen

[marxin at gcc dot gnu.org]

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

--- Comment #13 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to ncm from comment #12)
> As it is, your probability of failure in 9 and 10 is exactly 1.0.

I don't get this?

We speak a possibility that we break a stable release branch by a backport that
can somehow interact with other parts of the compiler. The purpose of stable
branches is to keep them as stable as possible.

This bug is a minor performance issue, not any wrong code or a compiler crash.