On Fri, Apr 5, 2024 at 8:14 PM Andrew Pinski <pins...@gmail.com> wrote:
>
> On Fri, Apr 5, 2024 at 5:28 AM 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.
>
> I do think we should have something similar to re-running
> ssa-ifcombine but I think it should be much later, like after the loop
> optimizations are done.
> Maybe just a simplified version of it (that does the combining and not
> the optimizations part) included in isel or pass_optimize_widening_mul
> (which itself should most likely become part of isel or renamed since
> it handles more than just widening multiply these days).
I've long wished we had a (late?) pass that can also undo if-conversion
(basically do what RTL expansion would later do). Maybe
gimple-predicate-analysis.cc (what's used by uninit analysis) can
represent mixed CFG + if-converted conditions so we can optimize
it and code-gen the condition in a more optimal manner much like
we have if-to-switch, switch-conversion and switch-expansion.
That said, I agree that re-running ifcombine should be later. And there's
still the old task of splitting tail-merging from PRE (and possibly making
it more effective).
Richard.
>
> Thanks,
> Andrew Pinski
>
>
> >
> > 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
> > +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 */);
> > 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" } */
> > -
> > 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
> >
