[Bug tree-optimization/56223] New: Integer ABS is not recognized for more complicated pattern

[Bug tree-optimization/56223] New: Integer ABS is not recognized for more complicated pattern

[ysrumyan at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56223

             Bug #: 56223
           Summary: Integer ABS is not recognized for more complicated
                    pattern
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: ysrumyan@gmail.com


In one important benchmark the following ABS pattern which is not recognized by
gcc was found:

   if (x >= 0)
      s += x;
   else
      s -= x;
that certainly should be converted to ABS as e.g. icc does.

In result the loop containing this pattern is not vectorized on x86 and arm.

I attached simple test-case to reproduce it.

If we complile it for atom with options " -O3 -m32 -march=atom2 -mtune=atom2
-ffast-math -msse2 -mfpmath=sse -ftree-vectorizer-verbose=2" we got:

...
t5.c:1: note: vectorized 0 loops in function.
...
t5.c:15: note: vectorized 1 loops in function.

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[ysrumyan at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56223

--- Comment #1 from Yuri Rumyantsev <ysrumyan at gmail dot com> 2013-02-06 11:41:21 UTC ---
Created attachment 29367
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29367
testcase to reproduce

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56223

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-02-06
     Ever Confirmed|0                           |1

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> 2013-02-06 12:14:54 UTC ---
Related support is in tree-ssa-phiopt.c which simply doesn't consider this
more complex pattern.  A changed pattern detection could detect


  if (v >= 0)
    use +v
    ...
  else
    use -v
    ...

  v unused outside the if

and replace it with

  v' = abs (v);
  if (v >= 0)
    use v'
    ...
  else
    use v'
    ...

and hope for further optimizations to detect the equivalences.

Or special-case a single addition / subtraction statement here.

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
There are ways of handling this case.  I think the second one is better than
the first.  The first is mentioned here.
The second would take:


if (a) {
  foo();
  b = d + e;
} else {
  goo();
  b = d -e;
}
and replace it with:
if (a) {
  foo();
  t = e;
} else {
  goo();
  t = -e;
}
b = d + t;
And then because in the original case foo and goo were non-existant, PHI-OPT
would replace the if statement with:
t = abs(e);
b = d + t;

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note factor_out_conditional_conversion is a special case comment #3 really.

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a start to this, though it needs to be fixed.  It only works for the
following extra case now:
int f(int a, int b, int c)
{
  int d0, d1, d;
    d1 = __builtin_abs(b);
  if (a) {
    d0 = __builtin_abs(c);
  d = d0;
  }  else {
  d = d1;
  }
  return d;
}

I also noticed that factor_out_conditional_conversion has a similar issue where
the cast is inside both if and else part.

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 50925
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50925&action=edit
Patch which starts to fix this

This is step 1 in fixing this bug, there are two or three more steps/patches.

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #5)
> I also noticed that factor_out_conditional_conversion has a similar issue
> where the cast is inside both if and else part.

I have a fix for that, though we don't remove one of the BB if it becomes
empty.

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |101205

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #7)
> (In reply to Andrew Pinski from comment #5)
> > I also noticed that factor_out_conditional_conversion has a similar issue
> > where the cast is inside both if and else part.
> 
> I have a fix for that, though we don't remove one of the BB if it becomes
> empty.

I have a patch for that but I ran into a code generation regression on aarch64,
PR 101205.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101205
[Bug 101205] csinv does not have an zero_extend version

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[pinskia at gcc dot gnu.org]

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

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note I did submit a patch
(https://gcc.gnu.org/pipermail/gcc-patches/2021-July/574892.html) for:
int abs0(int a, int b)
{
  int c = a - b;
  if (c <= 0) c = b - a;
  return c;
}
But this is unrelated to your original testcase.

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56223
Bug 56223 depends on bug 101205, which changed state.

Bug 101205 Summary: csinv does not have an zero_extend version
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101205

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

[Bug tree-optimization/56223] Integer ABS is not recognized for more complicated pattern

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED