Bug 97964 - Missed optimization opportunity for VRP
Summary: Missed optimization opportunity for VRP
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 10.2.0
: P3 normal
Target Milestone: 11.0
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks: VRP
  Show dependency treegraph
 
Reported: 2020-11-24 07:47 UTC by Ishiura Lab Compiler Team
Modified: 2021-08-14 22:29 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ishiura Lab Compiler Team 2020-11-24 07:47:48 UTC
We compiled two programs (A1.c, A2.c) by gcc-10.2.0 with -O3 option.
The two programs are equivalent, but resulted in different assembly codes.
Although varialbe a is volatile, the value of variable t can be 
determined at compile time via VRP optimization.  

Note that gcc-7.5.0 or older versions compiled the both code into the
same minimum assebmly codes.  For more precise: 

  gcc-10.2.0  diff
  gcc-9.3.0   diff
  gcc-8.0.1   diff
  gcc-7.5.0   OK
  gcc-6.5.0   OK
  gcc-5.5.0   OK


+---------------------------------+---------------------------------+
|                A1.c             |              A2.c               |
+---------------------------------+---------------------------------+
|int main (void)                  |int main (void)                  |
|{                                |{                                |
|  volatile int a = -1;           |  volatile int a = 1;            |
|  long b = -2;                   |                                 |
|                                 |                                 |
|  int c = a>0;                   |                                 |
|  int d = b*c;                   |                                 |
|  int e = 1-d;                   |  a;                             |
|  int t = (-1/(int)e)==1;        |  int t = 0;                     |
|                                 |                                 |
|  if (t != 0) __builtin_abort(); |  if (t != 0) __builtin_abort(); |
|                                 |                                 |
|  return 0;                      |  return 0;                      |
|}                                |}                                |
+---------------------------------+---------------------------------+

gcc-10.2.0
+-------------------------------+------------------------------+
| A1.s (gcc-10.2.0 A1.c -O3 -S) | A2.s (gcc-10.2.0 A2.c -O3 -S)|
+-------------------------------+------------------------------+
|main:                          |main:                         |
|.LFB0:                         |.LFB0:                        |
|   .cfi_startproc              |   .cfi_startproc             |
|   subq    $24, %rsp           |   movl    $1, -4(%rsp)       |
|   .cfi_def_cfa_offset 32      |   movl    -4(%rsp), %eax     |
|   movl    $1, 12(%ecx)        |                              |
|   movl    $-1, 12(%rsp)       |                              |
|   movl    12(%rsp), %eax      |                              |
|   testl   %eax, %eax          |                              |
|   setle   %al                 |                              |
|   movzbl  %al, %eax           |                              |
|   leal    -2(%rax,%rax), %eax |                              |
|   subl    %eax, %ecx          |                              |
|   movl    $-1, %eax           |                              |
|   cltd                        |                              |
|   idiv    %ecx                |                              |
|   cmpl    $1, %eax            |                              |
|   je      .L3                 |                              |
|   xorl    %eax, %eax          |   xorl    %eax, %eax         |
|   addq    $24, %rsp           |                              |
|   .cfi_def_cfa_offset 8       |                              |
|   ret                         |   ret                        |
|   .cfi_endproc                |   .cfi_endproc               |
|   .section   .text.unlikely   |                              |
|   .cfi_startproc              |                              |
|   .type   main.cold, @function|                              |
|main.cold:                     |                              |
|.LFSB0:                        |                              |
|.L3:                           |                              |
|    .cfi_def_cfa_offset 32     |                              |
|    call    abort              |                              |
|    .cfi_endproc               |                              |
|.LFE0:                         |.LFE0:                        |
|    .section  text.startup     |                              |
|    .size   main, .-main       |   .size   main, .-main       |
|    .section  .text.unlikely   |                              |
|    .size   main.cold, .-mai...|                              |
|.LCOLDE0:                      |                              |
|    .section  .text.startup    |                              |
|.LHOTE0:                       |                              |
|    .ident  "GCC:(GNU) 10.2.0" |   .ident  "GCC:(GNU) 10.2.0" |
|    .section  .note.GNU-stac...|   .section  .note.GNU-stac...|
+--------------------------------------------------------------+

gcc-7.5.0
+-------------------------------+------------------------------+
| A1.s (gcc-7.5.0 A1.c -O3 -S)  | A2.s (gcc-7.5.0 A2.c -O3 -S) |
+-------------------------------+------------------------------+
|main:                          |main:                         |
|.LFB0:                         |.LFB0:                        |
|    .cfi_startproc             |   .cfi_startproc             |
|    movl    $1, -4(%rsp)       |   movl    $1, -4(%rsp)       |
|    movl    -4(%rsp), %eax     |   movl    -4(%rsp), %eax     |
|    xorl    %eax, %eax         |   xorl    %eax, %eax         |
|    ret                        |   ret                        |
|    .cfi_endproc               |   .cfi_endproc               |
|.LFE0:                         |.LFE0:                        |
|    .size   main, .-main       |   .size   main, .-main       |
|    .ident  "GCC:(Ubuntu 7.5...|   .ident  "GCC:(Ubuntu 7.5...|
|    .section  .note.GNU-stac...|   .section  .note.GNU-stac...|
+--------------------------------------------------------------+

$ gcc -v
using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/x86_64-pc-linux-gnu/10.2.0/lto-wrapper
target: x86_64-pc-linux-gnu
configure woth: ../configure --enable-languages=c,c++ --prefix=/usr/local
--disable-bootstrap --disable-multilib
thred model: posix
Supported LTO compression algorithms: zlib
gcc version 10.2.0 (GCC) 

$ gcc-7 -v
Using built-in specs.
COLLECT_GCC=gcc-7
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/7/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 7.5.0-3ubuntu1~18.04'
--with-bugurl=file:///usr/share/doc/gcc-7/README.Bugs
--enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr
--with-gcc-major-version-only --program-suffix=-7 --program-prefix=x86_64-linux-gnu-
--enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext
--enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap
--enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify
--enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib
--with-target-system-zlib --enable-objc-gc=auto --enable-multiarch --disable-werror
--with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib
--with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver
--enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)
Comment 1 Andrew Pinski 2020-11-24 08:06:37 UTC
Here are the ranges that should be figured out:
c:[0,1]
d:[-2,0]
e:[0,2]

The problem comes from the fact -1/e could be undefined.
Comment 2 Richard Biener 2020-11-24 08:27:36 UTC
possibly caused by a bugfix
Comment 3 Jakub Jelinek 2020-11-24 09:32:23 UTC
Changed with r8-2090-g2071f8f980cc0de02af3d7d7de201f4f189058ff
Comment 4 Jakub Jelinek 2020-11-24 09:38:04 UTC
Note, trunk handles it fine again starting with r11-4755-g22984f3f090921b5ac80ec0057f6754ec458e97e
So I guess we should just add the testcase (perhaps use a parameter instead of volatile etc.) and close, ranger is not backportable and even smaller VRP improvements might be too risky.
Comment 5 Richard Biener 2020-11-24 09:39:14 UTC
(In reply to Jakub Jelinek from comment #4)
> Note, trunk handles it fine again starting with
> r11-4755-g22984f3f090921b5ac80ec0057f6754ec458e97e
> So I guess we should just add the testcase (perhaps use a parameter instead
> of volatile etc.) and close, ranger is not backportable and even smaller VRP
> improvements might be too risky.

Agreed.
Comment 6 GCC Commits 2020-11-24 09:44:10 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:a40d5772ff12a3a4f4830b7db27bedf54b617e8e

commit r11-5277-ga40d5772ff12a3a4f4830b7db27bedf54b617e8e
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Nov 24 10:42:56 2020 +0100

    testsuite: Add testcase for already fixed bug [PR97964]
    
    This testcase started failing with r8-2090 and works again starting
    with r11-4755.
    
    2020-11-24  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/97964
            * gcc.dg/tree-ssa/pr97964.c: New test.
Comment 7 Jakub Jelinek 2020-11-24 09:51:27 UTC
Thus fixed for 11.1+.