Bug 81503

Summary: [8 Regression] Wrong code at -O2
Product: gcc Reporter: Dmitry Babokin <babokin>
Component: tree-optimizationAssignee: Bill Schmidt <bill.schmidt>
Status: RESOLVED FIXED    
Severity: normal CC: bill.schmidt, jakub
Priority: P3 Keywords: wrong-code
Version: 8.0   
Target Milestone: 8.0   
See Also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112723
Host: Target:
Build: Known to work: 7.1.1
Known to fail: Last reconfirmed: 2017-07-21 00:00:00
Bug Depends on:    
Bug Blocks: 103035    
Attachments: Patch under test

Description Dmitry Babokin 2017-07-21 05:40:30 UTC
gcc trunk, rev250367, x86_64.

> cat f.cpp
#include <stdio.h>
unsigned short a = 41461;
unsigned short b = 3419;
int c = 0;

void foo() {
  if (a + b * ~(0 != 5))
    c = -~(b * ~(0 != 5)) + 2147483647;
}

int main() {
  foo();
  printf("%d\n", c);
  return 0;
}

> g++ f.cpp -O0; ./a.out
2147476810

> g++ f.cpp -O2; ./a.out
-2147483648
Comment 1 Marc Glisse 2017-07-21 06:55:16 UTC
Looks like SLSR does an overflow-unsafe transformation, then VRP2 takes advantage of it. Maybe.
Comment 2 Richard Biener 2017-07-21 07:22:23 UTC
Seems to work with GCC 7.
Comment 3 Martin Liška 2017-07-21 07:34:09 UTC
Started with r249447:

gcc/
	* match.pd (nop_convert): New predicate.
	((A +- CST1) +- CST2): Allow some NOP conversions.

gcc/testsuite/
	* gcc.dg/tree-ssa/addadd.c: Un-XFAIL.
	* gcc.dg/tree-ssa/addadd-2.c: New file.
Comment 4 Marc Glisse 2017-07-21 07:55:01 UTC
  if (a + b * -2)
    c = (b-1073741824)*-2;

might let you find an earlier culprit.
Comment 5 Bill Schmidt 2017-07-21 13:07:46 UTC
Mine, but my queue is getting pretty deep, so it will be a bit before I look at it.
Comment 6 Dmitry Babokin 2017-07-25 03:24:26 UTC
How can I switch off optimization phases to workaround the bug?

I have another instances of wrong code bugs, so I'd like to make sure that I don't create duplicate reports for the same problem.
Comment 7 Bill Schmidt 2017-07-25 13:07:25 UTC
Try -fno-slsr.
Comment 8 Bill Schmidt 2017-08-01 18:23:07 UTC
Patch under test that fixes this case:

Index: gcc/gimple-ssa-strength-reduction.c                                      
===================================================================
--- gcc/gimple-ssa-strength-reduction.c (revision 250791)                       
+++ gcc/gimple-ssa-strength-reduction.c (working copy)                          
@@ -2082,6 +2082,11 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
      types but allows for safe negation without twisted logic.  */             
   if (wi::fits_shwi_p (bump)                                                   
       && bump.to_shwi () != HOST_WIDE_INT_MIN                                  
+      /* It is more likely that the bump doesn't fit in the target             
+        type, so check whether constraining it to that type changes            
+        the value.  */                                                         
+      && wi::eq_p (TREE_INT_CST_LOW (wide_int_to_tree (target_type, bump)),    
+                  bump)                                                        
       /* It is not useful to replace casts, copies, negates, or adds of        
         an SSA name and a constant.  */                                        
       && cand_code != SSA_NAME
Comment 9 Bill Schmidt 2017-08-01 19:31:45 UTC
This is overkill, it has some test case fallout.  Will have to look a bit deeper.
Comment 10 Jakub Jelinek 2017-08-02 08:42:47 UTC
The TREE_INT_CST_LOW part looks suspicious.  Also, wide-int.h should provide enough infrastructure so that you should be able to do everything on wide-int, not have to create trees.
Comment 11 Bill Schmidt 2017-08-02 15:53:48 UTC
(In reply to Jakub Jelinek from comment #10)
> The TREE_INT_CST_LOW part looks suspicious.  Also, wide-int.h should provide
> enough infrastructure so that you should be able to do everything on
> wide-int, not have to create trees.

I just saw this comment now.  I think that the TREE_INT_CST_LOW is ok given that we've already constrained bump to fit in a HOST_WIDE_INT, right?  But anyway, your second point is well taken -- it looks like I can make use of wi::to_uhwi and wi::to_shwi with a specified precision from the target type, so use of trees should indeed be unnecessary.

I have a working patch that uses the trees, but I will rework that in favor of a pure wide-int solution.  Thanks for the suggestion!

Bill
Comment 12 Jakub Jelinek 2017-08-02 15:57:48 UTC
I had in mind something like
wi::eq_p (wi::ext (w, TYPE_PRECISION (type), TYPE_SIGN (type)), w)
or so.
Comment 13 Bill Schmidt 2017-08-02 16:08:17 UTC
(In reply to Jakub Jelinek from comment #12)
> I had in mind something like
> wi::eq_p (wi::ext (w, TYPE_PRECISION (type), TYPE_SIGN (type)), w)
> or so.

Ah, good, thank you.
Comment 14 Bill Schmidt 2017-08-02 20:38:16 UTC
Created attachment 41899 [details]
Patch under test

This is the patch I'm currently looking at.  I am unhappy at having to use a tree to get maxval, but I cannot find a way to convert a wide_int into a widest_int so that I can add them together (see the desired code that is commented out).  Do you have a suggestion how I can do this within the wide_int framework?
Comment 15 Bill Schmidt 2017-08-08 15:08:54 UTC
Proposed patch awaiting approval:  https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00347.html
Comment 16 Bill Schmidt 2017-08-29 14:42:25 UTC
Author: wschmidt
Date: Tue Aug 29 14:41:53 2017
New Revision: 251414

URL: https://gcc.gnu.org/viewcvs?rev=251414&root=gcc&view=rev
Log:
[gcc]

2017-08-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Jakub Jelinek  <jakub@redhat.com>
	    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81503
	* gimple-ssa-strength-reduction.c (replace_mult_candidate): Ensure
	folded constant fits in the target type; reorder tests for clarity.

[gcc/testsuite]

2017-08-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/81503
	* gcc.c-torture/execute/pr81503.c: New file.


Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr81503.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-ssa-strength-reduction.c
    trunk/gcc/testsuite/ChangeLog
Comment 17 Bill Schmidt 2017-08-29 14:52:52 UTC
Fixed in trunk so far.  Although this test case succeeds on GCC 7, the bug is latent there, so I'll keep this open and backport the fix to other releases in a week or so.
Comment 18 Bill Schmidt 2017-09-05 21:49:32 UTC
Author: wschmidt
Date: Tue Sep  5 21:49:01 2017
New Revision: 251743

URL: https://gcc.gnu.org/viewcvs?rev=251743&root=gcc&view=rev
Log:
[gcc]

2017-09-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	Backport from mainline
	2017-08-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
		    Jakub Jelinek  <jakub@redhat.com>
		    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81503
	* gimple-ssa-strength-reduction.c (replace_mult_candidate): Ensure
	folded constant fits in the target type; reorder tests for clarity.

[gcc/testsuite]

2017-09-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	Backport from mainline
	2017-08-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
		    Jakub Jelinek  <jakub@redhat.com>
		    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81503
	* gcc.c-torture/execute/pr81503.c: New file.


Added:
    branches/gcc-7-branch/gcc/testsuite/gcc.c-torture/execute/pr81503.c
Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/gimple-ssa-strength-reduction.c
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
Comment 19 Bill Schmidt 2017-09-05 21:51:09 UTC
Author: wschmidt
Date: Tue Sep  5 21:50:38 2017
New Revision: 251744

URL: https://gcc.gnu.org/viewcvs?rev=251744&root=gcc&view=rev
Log:
[gcc]

2017-09-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	Backport from mainline
	2017-08-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
		    Jakub Jelinek  <jakub@redhat.com>
		    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81503
	* gimple-ssa-strength-reduction.c (replace_mult_candidate): Ensure
	folded constant fits in the target type; reorder tests for clarity.

[gcc/testsuite]

2017-09-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	Backport from mainline
	2017-08-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
		    Jakub Jelinek  <jakub@redhat.com>
		    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81503
	* gcc.c-torture/execute/pr81503.c: New file.


Added:
    branches/gcc-6-branch/gcc/testsuite/gcc.c-torture/execute/pr81503.c
Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/gimple-ssa-strength-reduction.c
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
Comment 20 Bill Schmidt 2017-09-05 21:52:32 UTC
Author: wschmidt
Date: Tue Sep  5 21:52:01 2017
New Revision: 251745

URL: https://gcc.gnu.org/viewcvs?rev=251745&root=gcc&view=rev
Log:
[gcc]

2017-09-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	Backport from mainline
	2017-08-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
		    Jakub Jelinek  <jakub@redhat.com>
		    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81503
	* gimple-ssa-strength-reduction.c (replace_mult_candidate): Ensure
	folded constant fits in the target type; reorder tests for clarity.

[gcc/testsuite]

2017-09-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	Backport from mainline
	2017-08-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
		    Jakub Jelinek  <jakub@redhat.com>
		    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81503
	* gcc.c-torture/execute/pr81503.c: New file.


Added:
    branches/gcc-5-branch/gcc/testsuite/gcc.c-torture/execute/pr81503.c
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/gimple-ssa-strength-reduction.c
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
Comment 21 Bill Schmidt 2017-09-05 21:52:52 UTC
Fixed everywhere now.
Comment 22 Aldy Hernandez 2017-09-13 17:36:36 UTC
Author: aldyh
Date: Wed Sep 13 17:36:05 2017
New Revision: 252605

URL: https://gcc.gnu.org/viewcvs?rev=252605&root=gcc&view=rev
Log:
[gcc]

2017-08-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Jakub Jelinek  <jakub@redhat.com>
	    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81503
	* gimple-ssa-strength-reduction.c (replace_mult_candidate): Ensure
	folded constant fits in the target type; reorder tests for clarity.

[gcc/testsuite]

2017-08-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/81503
	* gcc.c-torture/execute/pr81503.c: New file.

Added:
    branches/range-gen2/gcc/testsuite/gcc.c-torture/execute/pr81503.c
Modified:
    branches/range-gen2/gcc/ChangeLog
    branches/range-gen2/gcc/gimple-ssa-strength-reduction.c
    branches/range-gen2/gcc/testsuite/ChangeLog