[Bug middle-end/63184] New: [4.8/4.9/5 Regression] Fails to simplify comparison

classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] New: [4.8/4.9/5 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

            Bug ID: 63184
           Summary: [4.8/4.9/5 Regression] Fails to simplify comparison
           Product: gcc
           Version: 4.9.1
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org

c-c++-common/pr19807-2.c

/* { dg-do link } */

extern void link_error(void);
int i;
int main()
{
  int a[4];
  if ((char*)&a[1] + 4*i + 4 != (char*)&a[i+2])
    link_error();
  return 0;
}

Fails with all optimization levels for all compilers.

c-c++-common/pr19807-3.c

/* { dg-do link } */

extern void link_error(void);
int i;
int main()
{
  int a[4];
  if (&a[1] + i + 1 != &a[i+2])
    link_error();
  return 0;
}

gcc 4.7 passes with -O[01] but fails with -O2+
gcc 4.8 and 4.9 fail with -O0 but passes with -O1+ (thanks to SLSR)
gcc 5.0 since the fix for PR63148 fails

which is a regression.

We don't seem to systematically use sth like the tree-affine.c framework
to simplify address comparisons and the fact that tree-reassoc.c doesn't
in any way associate pointer arithmetic doesn't help either.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [4.8/4.9/5 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2014-09-05
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
   Target Milestone|---                         |5.0
     Ever confirmed|0                           |1
           Severity|normal                      |minor

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Note that due to the inconsistency with -fstrict-overflow I don't consider this
an important optimization regression.

But I'll see if I can improve some generic code to handle these better (with
optimization).  But for 5.0 only.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [4.8/4.9/5 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ok, so we can (and do) forward addresses like &a[_9] into plain dereferences,
so lowering these addresses early is probably not a good idea.  We could
lower them as a very first thing in GIMPLE reassoc but that would still need
to apply transforms like (i + 1) * 4 -> i*4 + 4 to be most effective
(though that generally applies).

Otherwise I can't see an easy place to hook in simplification of

  i.0_3 = i;
  _4 = i.0_3 * 4;
  _5 = (sizetype) _4;
  _6 = _5 + 4;
  _7 = &a[1] + _6;
  _9 = i.0_3 + 2;
  _10 = &a[_9];
  if (_7 != _10)

which is what we get for the first testcase after initial scalar cleanups.
SLSR helps somewhat in some cases but it's run very late.

We could detect address comparisons with the same base and use the
affine combination machinery to simplify them though.  From somewhere.
tree-ssa-forwprop.c probably.  Simplify them to _7 - _10 CMP 0 (if
the addresses have the same base object at least that should cancel).
We canonicalize (base p+ off1) p+ off2 to base p+ (off1 + off2) which
should help here as well.

We could also only "fold" away the base object from the chain (not using
the affine machinery).  match-and-simplify doesn't help here because
the replacement involves an expression created by get_inner_reference
(well, a manual transform would work here, of course).
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [4.8/4.9/5 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [4.8/4.9/5/6 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.0                         |5.2

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 5.1 has been released.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [4.9/5/6 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.2                         |5.3

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 5.2 is being released, adjusting target milestone to 5.3.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [4.9/5/6 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ok, on trunk we now lower addresses somewhat but that doesn't help as we are
still faced with

  i.0_3 = i;
  _4 = (sizetype) i.0_3;
  _5 = _4 + 1;
  _6 = _5 * 4;
  _7 = &a[1] + _6;
  _8 = i.0_3 + 2;
  _12 = (sizetype) _8;
  _13 = _12 * 4;
  _9 = &a + _13;
  if (_7 != _9)

where FRE/PRE cannot see any equivalences, reassoc doesn't do anything and
SLSR does

-  _13 = _12 * 4;
+  _13 = _6 + 4;

but that's too late or rather DOM doesn't figure out the equivalence
of &a + _6 + 4 and &a[1] + _6 (not that I see how it could).
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [4.9/5/6 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.3                         |5.4

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 5.3 is being released, adjusting target milestone.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [4.9/5/6/7 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.4                         |5.5

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 5.4 is being released, adjusting target milestone.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [5/6/7 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com

--- Comment #8 from Jeffrey A. Law <law at redhat dot com> ---
So while we don't optimize this on the tree level, the addressing lowering that
was introduced for gcc-6 sets things up so the RTL optimizers can detect the
equivalences and remove the tests.

So ISTM we can do two things here.  Keep this as a regression since it'd be
nice for the SSA/gimple optimizers to catch this.  Or mark it was fixed for
gcc-6 and gcc-7 since the RTL optimizers are able to clean this stuff up.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [5/6/7 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |TREE

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Keep it open, it still is an issue in VN that I'd like to address.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [5/6/7/8 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
   Target Milestone|5.5                         |8.0
            Summary|[5/6/7 Regression] Fails to |[5/6/7/8 Regression] Fails
                   |simplify comparison         |to simplify comparison
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [6/7/8/9 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |deferred
   Target Milestone|8.0                         |9.0
            Summary|[6/7/8 Regression] Fails to |[6/7/8/9 Regression] Fails
                   |simplify comparison         |to simplify comparison

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
deferred.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [7/8/9 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

--- Comment #11 from Jeffrey A. Law <law at redhat dot com> ---
So could we reassociate the address arithmetic in match.pd so that we fold away
the pointer computation in favor of index adjustment in the ARRAY_REF?  Do we
have to worry about overflow in address reassociation?

Using the gimple from c#2:

  i.0_3 = i;
  _4 = i.0_3 * 4;
  _5 = (sizetype) _4;
  _6 = _5 + 4;
  _7 = &a[1] + _6;
  _9 = i.0_3 + 2;
  _10 = &a[_9];
  if (_7 != _10)


Transform it into:

  i.0_3 = i;
  temp = i.0_3 + 1 + 1;  /* +1 from pointer arith, +1 from array index ]
  _4 = i.0_3 * 4;        /* DEAD */
  _5 = (sizetype) _4;    /* DEAD */
  _6 = _5 + 4;           /* DEAD */
  _7 = &a[temp];
  _9 = i.0_3 + 2;
  _10 = &a[_9];
  if (_7 != _10)

That gives us a fighting chance to see that temp is equivalent to _9 and that
the ultimate addresses are equal.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [7/8/9 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |6.1.0
         Resolution|---                         |FIXED
   Target Milestone|9.0                         |6.0

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
It happens that we optimize both cases now (with optimization only).  Would be
still a regression vs. GCC 4.7 at -O0 but IMHO we shouldn't care about
optimizing this at -O0.

Thus fixed.

I'll add the testcases.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [7/8/9 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Wed Dec  5 14:55:59 2018
New Revision: 266827

URL: https://gcc.gnu.org/viewcvs?rev=266827&root=gcc&view=rev
Log:
2018-12-05  Richard Biener  <[hidden email]>

        PR middle-end/63184
        * c-c++-common/pr19807-2.c: New testcase.
        * c-c++-common/pr19807-3.c: Likewise.

Added:
    trunk/gcc/testsuite/c-c++-common/pr19807-2.c
    trunk/gcc/testsuite/c-c++-common/pr19807-3.c
Modified:
    trunk/gcc/testsuite/ChangeLog
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [7/8/9 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |ro at gcc dot gnu.org
         Resolution|FIXED                       |---

--- Comment #14 from Rainer Orth <ro at gcc dot gnu.org> ---
The new testcases FAIL on quite a number of targets (I'm seeing it on
sparc-sun-solaris2.11, 32 and 64-bit, but there are also gcc-testresults
postings
for aarch64-unknown-linux-gnu, armv8l-unknown-linux-gnueabihf,
ia64-suse-linux-gnu, moxie-unknown-elf,powerpc64-unknown-linux-gnu,
powerpc64le-unknown-linux-gnu:

+FAIL: c-c++-common/pr19807-2.c  -std=gnu++14 (test for excess errors)
+FAIL: c-c++-common/pr19807-2.c  -std=gnu++17 (test for excess errors)
+FAIL: c-c++-common/pr19807-2.c  -std=gnu++98 (test for excess errors)
+FAIL: c-c++-common/pr19807-3.c  -std=gnu++14 (test for excess errors)
+FAIL: c-c++-common/pr19807-3.c  -std=gnu++17 (test for excess errors)
+FAIL: c-c++-common/pr19807-3.c  -std=gnu++98 (test for excess errors)

+FAIL: c-c++-common/pr19807-2.c  -Wc++-compat  (test for excess errors)

Excess errors:
Undefined                       first referenced
 symbol                             in file
link_error                          /var/tmp//ccvTdBwc.o

+FAIL: c-c++-common/pr19807-3.c  -Wc++-compat  (test for excess errors)
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [7/8/9 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |NEW
   Last reconfirmed|2016-01-04 00:00:00         |2018-12-6

--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> ---
Oh, we only optimize it on RTL on x86, possibly with the help of combine and
complex addressing modes...  I'll adjust the testcase.
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [7/8/9 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|6.1.0                       |
   Target Milestone|6.0                         |7.5
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/63184] [7/8/9 Regression] Fails to simplify comparison

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63184

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

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

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
Now to answer Jeff my original idea was to change how VN handles addresses
to use tree-affine in there.  But that takes some time...

Given that we lower non-invariant addresses now another idea would be to
recognize

  _4 = _3 + 4;
  _5 = &a[1] + _4;

and reassociate that to combine the + 4 with the &a[1] (which is &a + 4).
Now the reassoc pass associates the constant last which makes this
unreliable to detect in a match.pd pattern (the constant may be far away)
but the reassoc pass itself could, when associating a chain, look for
whether the SSA name we start from is (single-)used in a POINTER_PLUS_EXPR
and the constant element in the reassoc ops[] array can be combined with
a constant offset in the address operand.

I'll see if that works out.
12