public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions
@ 2022-05-26 11:26 b.buschinski at googlemail dot com
  2022-05-30  9:57 ` [Bug tree-optimization/105740] " marxin at gcc dot gnu.org
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: b.buschinski at googlemail dot com @ 2022-05-26 11:26 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 105740
           Summary: missed optimization switch transformation for
                    conditions with duplicate conditions
           Product: gcc
           Version: 12.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: b.buschinski at googlemail dot com
  Target Milestone: ---

Created attachment 53037
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53037&action=edit
Attached code + asm from the compiler explorer link

Compiler Explorer Link: https://godbolt.org/z/q76s99Gj9

The code on the left does not generate a switch statement.
The code on the right does generate a switch statement.

The code works similar, the only difference is that the duplicate "f->len > 3"
check is moved to the top for the "right" version.

The compiler explorer code is actually a minimized version of the code I work
on (with way more conditions in a hot code path), where I can not easily move
the length check to the top, because the "f-> len > 3 && ..." is done in a very
complicated macro, but that's just details.

I expected both code versions to generate the same assembler.

Tested with GCC-12.1 and 11.3. 10.3 does not generate a switch version for both
versions, as only 11 got this nice feature.
On x86_64 Linux.


Please let me know if you need any additional details or if this report was
useful at all.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
@ 2022-05-30  9:57 ` marxin at gcc dot gnu.org
  2022-06-21  2:45 ` luoxhu at gcc dot gnu.org
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-05-30  9:57 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-05-30
             Status|UNCONFIRMED                 |NEW
                 CC|                            |marxin at gcc dot gnu.org

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Yes, I can confirm that at time of if-to-switch conversion pass, the condition
'len > 3' is not extracted and is present in every condition.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
  2022-05-30  9:57 ` [Bug tree-optimization/105740] " marxin at gcc dot gnu.org
@ 2022-06-21  2:45 ` luoxhu at gcc dot gnu.org
  2022-06-21  7:30 ` marxin at gcc dot gnu.org
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: luoxhu at gcc dot gnu.org @ 2022-06-21  2:45 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from luoxhu at gcc dot gnu.org ---
Run if_to_switch and convert_switch again after copyprop2 could remove the
redundant statement and expose opportunity for if-to-switch again, is this
reasonable or just move if-to-switch/switch-conversion later run only once?  


diff --git a/gcc/gimple-if-to-switch.cc b/gcc/gimple-if-to-switch.cc
index f7b0b02628b..8f55d0e2f75 100644
--- a/gcc/gimple-if-to-switch.cc
+++ b/gcc/gimple-if-to-switch.cc
@@ -484,6 +484,8 @@ public:
            || bit_test_cluster::is_enabled ());
   }

+  opt_pass *clone () { return new pass_if_to_switch (m_ctxt); }
+
   virtual unsigned int execute (function *);

 }; // class pass_if_to_switch
diff --git a/gcc/passes.def b/gcc/passes.def
index 375d3d62d51..b257307e085 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -243,6 +243,8 @@ along with GCC; see the file COPYING3.  If not see
         Clean them up.  Failure to do so well can lead to false
         positives from warnings for erroneous code.  */
       NEXT_PASS (pass_copy_prop);
+      NEXT_PASS (pass_if_to_switch);
+      NEXT_PASS (pass_convert_switch);
       /* Identify paths that should never be executed in a conforming
         program and isolate those paths.  */
       NEXT_PASS (pass_isolate_erroneous_paths);
diff --git a/gcc/tree-switch-conversion.cc b/gcc/tree-switch-conversion.cc
index 50a17927f39..d5c8262785e 100644
--- a/gcc/tree-switch-conversion.cc
+++ b/gcc/tree-switch-conversion.cc
@@ -2429,6 +2429,9 @@ public:

   /* opt_pass methods: */
   virtual bool gate (function *) { return flag_tree_switch_conversion != 0; }
+
+  opt_pass *clone () { return new pass_convert_switch (m_ctxt); }
+
   virtual unsigned int execute (function *);

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
  2022-05-30  9:57 ` [Bug tree-optimization/105740] " marxin at gcc dot gnu.org
  2022-06-21  2:45 ` luoxhu at gcc dot gnu.org
@ 2022-06-21  7:30 ` marxin at gcc dot gnu.org
  2022-06-21  9:02 ` rguenth at gcc dot gnu.org
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-06-21  7:30 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to luoxhu from comment #2)
> Run if_to_switch and convert_switch again after copyprop2 could remove the
> redundant statement and expose opportunity for if-to-switch again, is this
> reasonable or just move if-to-switch/switch-conversion later run only once?  

Dunno. There was a discussion about the pass placement in the original thread.
Richi, Jakub: What do you think about it?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
                   ` (2 preceding siblings ...)
  2022-06-21  7:30 ` marxin at gcc dot gnu.org
@ 2022-06-21  9:02 ` rguenth at gcc dot gnu.org
  2022-06-21  9:12 ` jakub at gcc dot gnu.org
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-06-21  9:02 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
IIRC switch conversion was run early originally because it was supposed to
improve inlining heuristics.  One might view if-to-switch + switch-conversion
as canonicalization which would mean running it before things like jump
threading or loop opts (most don't like switches at least).  Then one might
view if-to-switch + switch-conversion as (target specific) code generation
optimization (for that we also have pass_lower_switch, so one thing would
be re-running if-to-switch before that again, supposed pass_lower_switch
also performs the transforms switch-conversion does).

I usually don't like running things again and again.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
                   ` (3 preceding siblings ...)
  2022-06-21  9:02 ` rguenth at gcc dot gnu.org
@ 2022-06-21  9:12 ` jakub at gcc dot gnu.org
  2022-06-21  9:26 ` rguenther at suse dot de
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-06-21  9:12 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The problem with switch-conversion done multiple times is that when it is done
early, it can do worse job than when it is done late, e.g. we can have better
range information later which allows (unfortunately switch-conversion doesn't
use that yet, there is a PR about it) to ignore some never reachable values
etc.
So ideally we either need to be able to undo switch-conversion and redo it if
things have changed, or do it only late and for e.g. inlining costs perform it
only in analysis mode and record somewhere what kind of lowering would be done
and how much it would cost.
With multiple if-to-switch, don't we risk that we turn some ifs into switch,
then
switch-conversion lowers it back to ifs and then another if-to-switch matches
it again and again lowers it?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
                   ` (4 preceding siblings ...)
  2022-06-21  9:12 ` jakub at gcc dot gnu.org
@ 2022-06-21  9:26 ` rguenther at suse dot de
  2022-06-21  9:29 ` cvs-commit at gcc dot gnu.org
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: rguenther at suse dot de @ 2022-06-21  9:26 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 21 Jun 2022, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105740
> 
> --- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> The problem with switch-conversion done multiple times is that when it is done
> early, it can do worse job than when it is done late, e.g. we can have better
> range information later which allows (unfortunately switch-conversion doesn't
> use that yet, there is a PR about it) to ignore some never reachable values
> etc.
> So ideally we either need to be able to undo switch-conversion and redo it if
> things have changed, or do it only late and for e.g. inlining costs perform it
> only in analysis mode and record somewhere what kind of lowering would be done
> and how much it would cost.
> With multiple if-to-switch, don't we risk that we turn some ifs into switch,
> then
> switch-conversion lowers it back to ifs and then another if-to-switch matches
> it again and again lowers it?

Yeah, I think ideally switch conversion would be done as part of switch
lowering (plus maybe an extra if-to-switch).  The issue might be what
I said - some passes don't like switches, but they probably need to be
taught.  As of inline cost yes, doing likely-switch-converted analysis
would probably work.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
                   ` (5 preceding siblings ...)
  2022-06-21  9:26 ` rguenther at suse dot de
@ 2022-06-21  9:29 ` cvs-commit at gcc dot gnu.org
  2022-06-22  6:19 ` luoxhu at gcc dot gnu.org
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-06-21  9:29 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Xiong Hu Luo <luoxhu@gcc.gnu.org>:

https://gcc.gnu.org/g:57424087e82db140c06d4ea73f9700d5291c5bc2

commit r13-1184-g57424087e82db140c06d4ea73f9700d5291c5bc2
Author: Xionghu Luo <xionghuluo@tencent.com>
Date:   Thu Jun 9 15:46:30 2022 +0800

    if-to-switch: Don't skip the first condition bb when find_conditions in
if-to-switch [PR105740]

    The if condition is at last of first bb, so side effect statement in first
BB
    doesn't matter, then the first if condition could also be folded to switch
    table.

    gcc/ChangeLog:

            PR target/105740
            * gimple-if-to-switch.cc (find_conditions): Don't skip the first
            condition bb.

    gcc/testsuite/ChangeLog:

            PR target/105740
            * gcc.dg/tree-ssa/if-to-switch-11.c: New test.

    Signed-off-by: Xionghu Luo <xionghuluo@tencent.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
                   ` (6 preceding siblings ...)
  2022-06-21  9:29 ` cvs-commit at gcc dot gnu.org
@ 2022-06-22  6:19 ` luoxhu at gcc dot gnu.org
  2022-06-27 13:58 ` marxin at gcc dot gnu.org
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: luoxhu at gcc dot gnu.org @ 2022-06-22  6:19 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from luoxhu at gcc dot gnu.org ---
(In reply to rguenther@suse.de from comment #6)
> On Tue, 21 Jun 2022, jakub at gcc dot gnu.org wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105740
> > 
> > --- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> > The problem with switch-conversion done multiple times is that when it is done
> > early, it can do worse job than when it is done late, e.g. we can have better
> > range information later which allows (unfortunately switch-conversion doesn't
> > use that yet, there is a PR about it) to ignore some never reachable values
> > etc.
> > So ideally we either need to be able to undo switch-conversion and redo it if
> > things have changed, or do it only late and for e.g. inlining costs perform it
> > only in analysis mode and record somewhere what kind of lowering would be done
> > and how much it would cost.
> > With multiple if-to-switch, don't we risk that we turn some ifs into switch,
> > then
> > switch-conversion lowers it back to ifs and then another if-to-switch matches
> > it again and again lowers it?
> 
> Yeah, I think ideally switch conversion would be done as part of switch
> lowering (plus maybe an extra if-to-switch).  The issue might be what
> I said - some passes don't like switches, but they probably need to be
> taught.  As of inline cost yes, doing likely-switch-converted analysis
> would probably work.

git diff
diff --git a/gcc/passes.def b/gcc/passes.def
index b257307e085..1376e7cb28d 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -243,8 +243,6 @@ along with GCC; see the file COPYING3.  If not see
         Clean them up.  Failure to do so well can lead to false
         positives from warnings for erroneous code.  */
       NEXT_PASS (pass_copy_prop);
       /* Identify paths that should never be executed in a conforming
         program and isolate those paths.  */
       NEXT_PASS (pass_isolate_erroneous_paths);
@@ -329,6 +327,7 @@ along with GCC; see the file COPYING3.  If not see
       POP_INSERT_PASSES ()
       NEXT_PASS (pass_simduid_cleanup);
       NEXT_PASS (pass_lower_vector_ssa);
+      NEXT_PASS (pass_if_to_switch);
       NEXT_PASS (pass_lower_switch);
       NEXT_PASS (pass_cse_reciprocals);
       NEXT_PASS (pass_reassoc, false /* early_p */);

Tried this to add the second if_to_switch before lower_switch, but switch
lowering doesn't work same as switch_conversion:

;; Function test2 (test2, funcdef_no=0, decl_uid=1982, cgraph_uid=1,
symbol_order=0)

beginning to process the following SWITCH statement ((null):0) : -------
switch (_2) <default: <L27> [INV], case 1: <L20> [INV], case 2: <L21> [INV],
case 3: <L22> [INV], case 4: <L2
3> [INV], case 5: <L24> [INV], case 6: <L25> [INV]>

;; GIMPLE switch case clusters: JT(values:6 comparisons:6 range:6 density:
100.00%):1-6
Removing basic block 11
;; basic block 11, loop depth 0
;;  pred:
switch (_2) <default: <L27> [INV], case 1: <L20> [INV], case 2: <L21> [INV],
case 3: <L22> [INV], case 4: <L2
3> [INV], case 5: <L24> [INV], case 6: <L25> [INV]>
;;  succ:       4
;;              5
;;              6
;;              7
;;              8
;;              9
;;              10



Updating SSA:
Registering new PHI nodes in block #0
Registering new PHI nodes in block #2
Updating SSA information for statement _1 = f_10(D)->len;
Registering new PHI nodes in block #3
Updating SSA information for statement _2 = f_10(D)->arr[3];
...
int test2 (struct fs * f)
{
  int _1;
  int _2;
  int _8;

  <bb 2> [local count: 1073741824]:
  _1 = f_10(D)->len;
  if (_1 > 3)
    goto <bb 3>; [50.00%]
  else
    goto <bb 10>; [50.00%]

  <bb 3> [local count: 536870913]:
  _2 = f_10(D)->arr[3];
  switch (_2) <default: <L27> [0.00%], case 1: <L20> [16.67%], case 2: <L21>
[16.67%], case 3: <L22> [16.67%], case 4: <L23> [16.67%], case 5: <L24>
[16.67%], case 6: <L25> [16.67%]>

  <bb 4> [local count: 67108864]:
<L20>:
  goto <bb 10>; [100.00%]

  <bb 5> [local count: 62914560]:
<L21>:
  goto <bb 10>; [100.00%]

  <bb 6> [local count: 58982400]:
<L22>:
  goto <bb 10>; [100.00%]

  <bb 7> [local count: 55296000]:
<L23>:
  goto <bb 10>; [100.00%]

  <bb 8> [local count: 51840000]:
<L24>:
  goto <bb 10>; [100.00%]

  <bb 9> [local count: 48600000]:
<L25>:

  <bb 10> [local count: 1073741824]: 
 # _8 = PHI <12(4), 27(5), 38(6), 18(7), 58(8), 68(9), 0(3), 0(2)>
<L27>:
  return _8;

}

ASM still contains indirect jump table like -fno-switch-conversion:

test2:
.LFB0:
        .cfi_startproc
        xorl    %eax, %eax
        cmpl    $3, (%rdi)
        jle     .L1
        cmpl    $6, 16(%rdi)
        ja      .L3
        movl    16(%rdi), %eax
        jmp     *.L5(,%rax,8)
        .section        .rodata
        .align 8
        .align 4
.L5:
        .quad   .L3
        .quad   .L11
        .quad   .L9
        .quad   .L8
        .quad   .L7
        .quad   .L6
        .quad   .L4
        .text
        .p2align 4,,10
        .p2align 3
.L11:
        movl    $12, %eax
.L1:
        ret
        .p2align 4,,10
        .p2align 3
.L9:
        movl    $27, %eax
        ret
        .p2align 4,,10
        .p2align 3
.L8:
        movl    $38, %eax
        ret
        .p2align 4,,10
        .p2align 3
.L7:
        movl    $18, %eax
        ret
        .p2align 4,,10
        .p2align 3
.L6:
        movl    $58, %eax
        ret
        .p2align 4,,10
        .p2align 3
.L4:
        movl    $68, %eax
        ret
.L3:
        xorl    %eax, %eax
        ret
        .cfi_endproc
.LFE0:
        .size   test2, .-test2



Is this bug of lower_switch or expected?  From the code, they have different
purpose as switch_conversion turns switch to single if-else while lower_switch
expand CLUSTERS as a decision tree.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
                   ` (7 preceding siblings ...)
  2022-06-22  6:19 ` luoxhu at gcc dot gnu.org
@ 2022-06-27 13:58 ` marxin at gcc dot gnu.org
  2022-07-01  2:06 ` luoxhu at gcc dot gnu.org
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-06-27 13:58 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to luoxhu from comment #8)
> (In reply to rguenther@suse.de from comment #6)
> > On Tue, 21 Jun 2022, jakub at gcc dot gnu.org wrote:
> > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105740
> > > 
> > > --- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> > > The problem with switch-conversion done multiple times is that when it is done
> > > early, it can do worse job than when it is done late, e.g. we can have better
> > > range information later which allows (unfortunately switch-conversion doesn't
> > > use that yet, there is a PR about it) to ignore some never reachable values
> > > etc.
> > > So ideally we either need to be able to undo switch-conversion and redo it if
> > > things have changed, or do it only late and for e.g. inlining costs perform it
> > > only in analysis mode and record somewhere what kind of lowering would be done
> > > and how much it would cost.
> > > With multiple if-to-switch, don't we risk that we turn some ifs into switch,
> > > then
> > > switch-conversion lowers it back to ifs and then another if-to-switch matches
> > > it again and again lowers it?
> > 
> > Yeah, I think ideally switch conversion would be done as part of switch
> > lowering (plus maybe an extra if-to-switch).  The issue might be what
> > I said - some passes don't like switches, but they probably need to be
> > taught.  As of inline cost yes, doing likely-switch-converted analysis
> > would probably work.
> 
> git diff
> diff --git a/gcc/passes.def b/gcc/passes.def
> index b257307e085..1376e7cb28d 100644
> --- a/gcc/passes.def
> +++ b/gcc/passes.def
> @@ -243,8 +243,6 @@ along with GCC; see the file COPYING3.  If not see
>          Clean them up.  Failure to do so well can lead to false
>          positives from warnings for erroneous code.  */
>        NEXT_PASS (pass_copy_prop);
>        /* Identify paths that should never be executed in a conforming
>          program and isolate those paths.  */
>        NEXT_PASS (pass_isolate_erroneous_paths);
> @@ -329,6 +327,7 @@ along with GCC; see the file COPYING3.  If not see
>        POP_INSERT_PASSES ()
>        NEXT_PASS (pass_simduid_cleanup);
>        NEXT_PASS (pass_lower_vector_ssa);
> +      NEXT_PASS (pass_if_to_switch);
>        NEXT_PASS (pass_lower_switch);
>        NEXT_PASS (pass_cse_reciprocals);
>        NEXT_PASS (pass_reassoc, false /* early_p */);
> 
> Tried this to add the second if_to_switch before lower_switch, but switch
> lowering doesn't work same as switch_conversion:

Note the lowering expand to a decision tree where node of such tree can be
jump-tables,
bit-tests or simple comparisons.

> 
> ;; Function test2 (test2, funcdef_no=0, decl_uid=1982, cgraph_uid=1,
> symbol_order=0)
> 
> beginning to process the following SWITCH statement ((null):0) : -------
> switch (_2) <default: <L27> [INV], case 1: <L20> [INV], case 2: <L21> [INV],
> case 3: <L22> [INV], case 4: <L2
> 3> [INV], case 5: <L24> [INV], case 6: <L25> [INV]>
> 
> ;; GIMPLE switch case clusters: JT(values:6 comparisons:6 range:6 density:
> 100.00%):1-6

So jump-table is selected. Where do you see this GIMPLE representation?

...

> 
> ASM still contains indirect jump table like -fno-switch-conversion:

> 
> Is this bug of lower_switch or expected?

What bug do you mean? 

> From the code, they have different
> purpose as switch_conversion turns switch to single if-else while

No switch_conversion expands a switch statement to a series of assignment
based on CSWITCH[index] arrays.

> lower_switch expand CLUSTERS as a decision tree.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
                   ` (8 preceding siblings ...)
  2022-06-27 13:58 ` marxin at gcc dot gnu.org
@ 2022-07-01  2:06 ` luoxhu at gcc dot gnu.org
  2022-07-08 10:46 ` marxin at gcc dot gnu.org
  2023-06-26  6:43 ` b.buschinski at googlemail dot com
  11 siblings, 0 replies; 13+ messages in thread
From: luoxhu at gcc dot gnu.org @ 2022-07-01  2:06 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from luoxhu at gcc dot gnu.org ---
(In reply to Martin Liška from comment #9)
> (In reply to luoxhu from comment #8)
> > (In reply to rguenther@suse.de from comment #6)
> > > On Tue, 21 Jun 2022, jakub at gcc dot gnu.org wrote:
> > > 
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105740
> > > > 
> > > > --- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> > > > The problem with switch-conversion done multiple times is that when it is done
> > > > early, it can do worse job than when it is done late, e.g. we can have better
> > > > range information later which allows (unfortunately switch-conversion doesn't
> > > > use that yet, there is a PR about it) to ignore some never reachable values
> > > > etc.
> > > > So ideally we either need to be able to undo switch-conversion and redo it if
> > > > things have changed, or do it only late and for e.g. inlining costs perform it
> > > > only in analysis mode and record somewhere what kind of lowering would be done
> > > > and how much it would cost.
> > > > With multiple if-to-switch, don't we risk that we turn some ifs into switch,
> > > > then
> > > > switch-conversion lowers it back to ifs and then another if-to-switch matches
> > > > it again and again lowers it?
> > > 
> > > Yeah, I think ideally switch conversion would be done as part of switch
> > > lowering (plus maybe an extra if-to-switch).  The issue might be what
> > > I said - some passes don't like switches, but they probably need to be
> > > taught.  As of inline cost yes, doing likely-switch-converted analysis
> > > would probably work.
> > 
> > git diff
> > diff --git a/gcc/passes.def b/gcc/passes.def
> > index b257307e085..1376e7cb28d 100644
> > --- a/gcc/passes.def
> > +++ b/gcc/passes.def
> > @@ -243,8 +243,6 @@ along with GCC; see the file COPYING3.  If not see
> >          Clean them up.  Failure to do so well can lead to false
> >          positives from warnings for erroneous code.  */
> >        NEXT_PASS (pass_copy_prop);
> >        /* Identify paths that should never be executed in a conforming
> >          program and isolate those paths.  */
> >        NEXT_PASS (pass_isolate_erroneous_paths);
> > @@ -329,6 +327,7 @@ along with GCC; see the file COPYING3.  If not see
> >        POP_INSERT_PASSES ()
> >        NEXT_PASS (pass_simduid_cleanup);
> >        NEXT_PASS (pass_lower_vector_ssa);
> > +      NEXT_PASS (pass_if_to_switch);
> >        NEXT_PASS (pass_lower_switch);
> >        NEXT_PASS (pass_cse_reciprocals);
> >        NEXT_PASS (pass_reassoc, false /* early_p */);
> > 
> > Tried this to add the second if_to_switch before lower_switch, but switch
> > lowering doesn't work same as switch_conversion:
> 
> Note the lowering expand to a decision tree where node of such tree can be
> jump-tables,
> bit-tests or simple comparisons.
> 
> > 
> > ;; Function test2 (test2, funcdef_no=0, decl_uid=1982, cgraph_uid=1,
> > symbol_order=0)
> > 
> > beginning to process the following SWITCH statement ((null):0) : -------
> > switch (_2) <default: <L27> [INV], case 1: <L20> [INV], case 2: <L21> [INV],
> > case 3: <L22> [INV], case 4: <L2
> > 3> [INV], case 5: <L24> [INV], case 6: <L25> [INV]>
> > 
> > ;; GIMPLE switch case clusters: JT(values:6 comparisons:6 range:6 density:
> > 100.00%):1-6
> 
> So jump-table is selected. Where do you see this GIMPLE representation?

This is dumped by the second run of iftoswitch after fre5.

> 
> ...
> 
> > 
> > ASM still contains indirect jump table like -fno-switch-conversion:
> 
> > 
> > Is this bug of lower_switch or expected?
> 
> What bug do you mean? 

Sorry, it not a bug, got to know that switch lower and switch conversion are
doing two different things, different with "pass_lower_switch
also performs the transforms switch-conversion does" in c#4?

> 
> > From the code, they have different
> > purpose as switch_conversion turns switch to single if-else while
> 
> No switch_conversion expands a switch statement to a series of assignment
> based on CSWITCH[index] arrays.
> 
> > lower_switch expand CLUSTERS as a decision tree.

Yes, rerun pass_convert_switch after the second if_to_switch could generate the
CSWITCH[index]. 

pr105740.c.195t.switchconv2:

  <bb 2> [local count: 1073741824]:
  if (x_4(D) > 3)
    goto <bb 3>; [50.00%]
  else
    goto <bb 6>; [50.00%]

  <bb 3> [local count: 536870913]:
  _1 = f_6(D)->arr[3];
  _10 = (unsigned int) _1;
  _2 = _10 + 4294967295;
  if (_2 <= 5)
    goto <bb 5>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 4> [local count: 1073741822]:
<L28>:
  _8 = 0;
  goto <bb 6>; [100.00%]

  <bb 5> [local count: 1073741822]:
<L29>:
  _9 = CSWTCH.4[_2];

  <bb 6> [local count: 2147483644]:
  # _3 = PHI <_8(4), 0(2), _9(5)>
<L30>:
<L27>:
  return _3;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
                   ` (9 preceding siblings ...)
  2022-07-01  2:06 ` luoxhu at gcc dot gnu.org
@ 2022-07-08 10:46 ` marxin at gcc dot gnu.org
  2023-06-26  6:43 ` b.buschinski at googlemail dot com
  11 siblings, 0 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-07-08 10:46 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Martin Liška <marxin at gcc dot gnu.org> ---
> Sorry, it not a bug, got to know that switch lower and switch conversion are
> doing two different things, different with "pass_lower_switch
> also performs the transforms switch-conversion does" in c#4?

Yes, there are 2 different transformations and Richi was not correct, because
switch-conversion (creation of a static array like CSWTCH.foo.bar) is not
done by switch-lowering pass.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug tree-optimization/105740] missed optimization switch transformation for conditions with duplicate conditions
  2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
                   ` (10 preceding siblings ...)
  2022-07-08 10:46 ` marxin at gcc dot gnu.org
@ 2023-06-26  6:43 ` b.buschinski at googlemail dot com
  11 siblings, 0 replies; 13+ messages in thread
From: b.buschinski at googlemail dot com @ 2023-06-26  6:43 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from Bernd Buschinski <b.buschinski at googlemail dot com> ---
Hi, according to godbolt (gcc trunk) this is still present.

is there anything I can do to help here?

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-06-26  6:43 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-26 11:26 [Bug tree-optimization/105740] New: missed optimization switch transformation for conditions with duplicate conditions b.buschinski at googlemail dot com
2022-05-30  9:57 ` [Bug tree-optimization/105740] " marxin at gcc dot gnu.org
2022-06-21  2:45 ` luoxhu at gcc dot gnu.org
2022-06-21  7:30 ` marxin at gcc dot gnu.org
2022-06-21  9:02 ` rguenth at gcc dot gnu.org
2022-06-21  9:12 ` jakub at gcc dot gnu.org
2022-06-21  9:26 ` rguenther at suse dot de
2022-06-21  9:29 ` cvs-commit at gcc dot gnu.org
2022-06-22  6:19 ` luoxhu at gcc dot gnu.org
2022-06-27 13:58 ` marxin at gcc dot gnu.org
2022-07-01  2:06 ` luoxhu at gcc dot gnu.org
2022-07-08 10:46 ` marxin at gcc dot gnu.org
2023-06-26  6:43 ` b.buschinski at googlemail dot com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).