On Fri, Apr 5, 2024 at 2:28 PM Manolis Tsamis <manolis.tsa...@vrull.eu> wrote:
>
> If we consider code like:
>
> if (bar1 == x)
> return foo();
> if (bar2 != y)
> return foo();
> return 0;
>
> We would like the ifcombine pass to convert this to:
>
> if (bar1 == x || bar2 != y)
> return foo();
> return 0;
>
> The ifcombine pass can handle this transformation but it is ran very early and
> it misses the opportunity because there are two seperate blocks for foo().
> The pre pass is good at removing duplicate code and blocks and due to that
> running ifcombine again after it can increase the number of successful
> conversions.
>
> PR 102793
>
> gcc/ChangeLog:
>
> * common.opt: -ftree-ifcombine option, enabled by default.
> * doc/invoke.texi: Document.
> * passes.def: Re-run ssa-ifcombine after pre.
> * tree-ssa-ifcombine.cc: Make ifcombine cloneable. Add gate function.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/20030922-2.c: Change flag to -fno-tree-ifcombine.
> * gcc.dg/uninit-pred-6_c.c: Remove inconsistent check.
> * gcc.target/aarch64/pr102793.c: New test.
>
> Signed-off-by: Manolis Tsamis <manolis.tsa...@vrull.eu>
> ---
>
> gcc/common.opt | 4 +++
> gcc/doc/invoke.texi | 5 ++++
> gcc/passes.def | 1 +
> gcc/testsuite/gcc.dg/tree-ssa/20030922-2.c | 2 +-
> gcc/testsuite/gcc.dg/uninit-pred-6_c.c | 4 ---
> gcc/testsuite/gcc.target/aarch64/pr102793.c | 30 +++++++++++++++++++++
> gcc/tree-ssa-ifcombine.cc | 5 ++++
> 7 files changed, 46 insertions(+), 5 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr102793.c
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index ad348844775..e943202bcf1 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -3163,6 +3163,10 @@ ftree-phiprop
> Common Var(flag_tree_phiprop) Init(1) Optimization
> Enable hoisting loads from conditional pointers.
>
> +ftree-ifcombine
Please don't add further -ftree-X flags, 'tree' means nothing
to users. -fif-combine would be better.
> +Common Var(flag_tree_ifcombine) Init(1) Optimization
> +Merge some conditional branches to simplify control flow.
> +
> ftree-pre
> Common Var(flag_tree_pre) Optimization
> Enable SSA-PRE optimization on trees.
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index e2edf7a6c13..8d2ff6b4512 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -13454,6 +13454,11 @@ This flag is enabled by default at @option{-O1} and
> higher.
> Perform hoisting of loads from conditional pointers on trees. This
> pass is enabled by default at @option{-O1} and higher.
>
> +@opindex ftree-ifcombine
> +@item -ftree-ifcombine
> +Merge some conditional branches to simplify control flow. This pass
> +is enabled by default at @option{-O1} and higher.
> +
> @opindex fhoist-adjacent-loads
> @item -fhoist-adjacent-loads
> Speculatively hoist loads from both branches of an if-then-else if the
> diff --git a/gcc/passes.def b/gcc/passes.def
> index 1cbbd413097..1765b476131 100644
> --- a/gcc/passes.def
> +++ b/gcc/passes.def
> @@ -270,6 +270,7 @@ along with GCC; see the file COPYING3. If not see
> NEXT_PASS (pass_lim);
> NEXT_PASS (pass_walloca, false);
> NEXT_PASS (pass_pre);
> + NEXT_PASS (pass_tree_ifcombine);
> NEXT_PASS (pass_sink_code, false /* unsplit edges */);
Please move it here, after sinking.
> NEXT_PASS (pass_sancov);
> NEXT_PASS (pass_asan);
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/20030922-2.c
> b/gcc/testsuite/gcc.dg/tree-ssa/20030922-2.c
> index 16c79da9521..66c9f481a2f 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/20030922-2.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/20030922-2.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-O1 -fdump-tree-dom2 -fdisable-tree-ifcombine" } */
> +/* { dg-options "-O1 -fdump-tree-dom2 -fno-tree-ifcombine" } */
>
> struct rtx_def;
> typedef struct rtx_def *rtx;
> diff --git a/gcc/testsuite/gcc.dg/uninit-pred-6_c.c
> b/gcc/testsuite/gcc.dg/uninit-pred-6_c.c
> index f60868dad23..2d8e6501a45 100644
> --- a/gcc/testsuite/gcc.dg/uninit-pred-6_c.c
> +++ b/gcc/testsuite/gcc.dg/uninit-pred-6_c.c
> @@ -20,10 +20,6 @@ int foo (int n, int l, int m, int r)
> if ( (n > 10) && l)
> blah(v); /* { dg-bogus "uninitialized" "bogus warning" } */
>
> - if (l)
> - if (n > 12)
> - blah(v); /* { dg-bogus "uninitialized" "bogus warning" } */
> -
What's "inconsistent" about this check? I suppose we now diagnose this?
The appropriate way would be to XFAIL this but I'd like you to explain
why we now diagnose this (I don't see obvious if-combining opportunities).
On a general note you rely on the tail-merging pass which is part of PRE
and which hasn't seen any love and which isn't very powerful either. I'm not
sure it's worth doing if-combining on the whole IL again because of it.
It might be possible to locally try if-combining from the immediate dominator
of a merged tail from inside tail-merging itself?
Richard.
> return 0;
> }
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr102793.c
> b/gcc/testsuite/gcc.target/aarch64/pr102793.c
> new file mode 100644
> index 00000000000..78d48e01637
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr102793.c
> @@ -0,0 +1,30 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +typedef unsigned long uint64_t;
> +
> +int ccmp1(uint64_t* s1, uint64_t* s2, int(*foo)(void))
> +{
> + uint64_t d1, d2, bar;
> + d1 = *s1++;
> + d2 = *s2++;
> + bar = (d1 + d2) & 0xabcd;
> + if (bar == 0 || d1 != d2)
> + return foo();
> + return 0;
> +}
> +
> +int ccmp2(uint64_t* s1, uint64_t* s2, int(*foo)(void))
> +{
> + uint64_t d1, d2, bar;
> + d1 = *s1++;
> + d2 = *s2++;
> + bar = (d1 + d2) & 0xabcd;
> + if (bar == 0)
> + return foo();
> + if (d1 != d2)
> + return foo();
> + return 0;
> +}
> +
> +/* { dg-final { scan-assembler-times "ccmp\t" 2 } } */
> \ No newline at end of file
> diff --git a/gcc/tree-ssa-ifcombine.cc b/gcc/tree-ssa-ifcombine.cc
> index 6a3bc99190d..0bf9fe8b692 100644
> --- a/gcc/tree-ssa-ifcombine.cc
> +++ b/gcc/tree-ssa-ifcombine.cc
> @@ -838,6 +838,11 @@ public:
> {}
>
> /* opt_pass methods: */
> + opt_pass * clone () final override
> + {
> + return new pass_tree_ifcombine (m_ctxt);
> + }
> + bool gate (function *) final override { return flag_tree_ifcombine; }
> unsigned int execute (function *) final override;
>
> }; // class pass_tree_ifcombine
> --
> 2.44.0
>
