Bug 70333 - [5 Regression] Test miscompiled with -O0.
Summary: [5 Regression] Test miscompiled with -O0.
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: 5.4
Assignee: Richard Biener
URL:
Keywords: wrong-code
Depends on: 70450
Blocks: yarpgen
  Show dependency treegraph
 
Reported: 2016-03-21 06:28 UTC by Vsevolod Livinskii
Modified: 2021-11-01 23:07 UTC (History)
1 user (show)

See Also:
Host:
Target: x86_64-*-*
Build:
Known to work: 6.0
Known to fail: 5.3.0
Last reconfirmed: 2016-03-21 00:00:00


Attachments
Reproducer. (215 bytes, text/x-csrc)
2016-03-21 06:28 UTC, Vsevolod Livinskii
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vsevolod Livinskii 2016-03-21 06:28:36 UTC
Created attachment 38042 [details]
Reproducer.

Test case produces incorrect result with -O0. It also fails with gcc version 5.3.1 20160318 (Revision=234347) and works fine with gcc version 4.9.4 20160318 (prerelease) (Revision=234347)

GCC version:
> g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/export/users/vlivinsk/gcc-trunk/bin/libexec/gcc/x86_64-pc-linux-gnu/6.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /export/users/vlivinsk/gcc-trunk/gcc/configure --with-arch=corei7 --with-cpu=corei7 --enable-clocale=gnu --with-system-zlib --enable-shared --with-demangler-in-ld --enable-cloog-backend=isl --with-fpmath=sse --enable-checking=release --enable-languages=c,c++,lto --with-gmp=/export/users/vlivinsk/gcc-trunk/gmp-6.1.0/bin --with-mpfr=/export/users/vlivinsk/gcc-trunk/mpfr-3.1.3/bin --with-mpc=/export/users/vlivinsk/gcc-trunk/mpc-1.0.3/bin --prefix=/export/users/vlivinsk/gcc-trunk/bin
Thread model: posix
gcc version 6.0.0 20160319 (experimental) (Revision=234350)

Test:
#include <iostream>

signed char a = -33;
signed char b = -60;
signed char c = 70;
long int d = 8591990532774389055L;

void foo () {
    signed char e = ((unsigned long int)a < (unsigned long int)8) == (a > c);
    d = ((2UL * b) * (e * 13)) * (32 << 24);
}

int main () {
    foo ();
    if (d != -837518622720)
        abort();
    return 0;
}
Comment 1 Richard Biener 2016-03-21 09:19:39 UTC
Confirmed.

--- t.c.003t.original   2016-03-21 10:17:23.438078512 +0100
+++ t.c.003t.original.x 2016-03-21 10:17:17.694014323 +0100
@@ -7,7 +7,7 @@
   signed char e = (signed char) ((unsigned char) a > 7 ^ a > c);
 
     signed char e = (signed char) ((unsigned char) a > 7 ^ a > c);
-  d = (long int) (((long unsigned int) b * (long unsigned int) e) * 13958643712);
+  d = (long int) (((long unsigned int) b * (long unsigned int) e) * 18446744070488326144);
 }

smells like another wide-int related bug.
Comment 2 Jakub Jelinek 2016-03-21 11:15:18 UTC
As expected, started with r210113.
Comment 3 Jakub Jelinek 2016-03-21 11:33:03 UTC
Simplified testcase:
unsigned long int
foo (signed char b, signed char e)
{
  return ((2ULL * b) * (e * 13)) * (32 << 24);
}

int
main ()
{
  if (__CHAR_BIT__ == 8
      && sizeof (int) == 4
      && sizeof (long long) == 8
      && foo (-60, 1) != 0xffffff3d00000000ULL)
    __builtin_abort ();
  return 0;
}
Comment 4 Richard Biener 2016-03-21 11:38:30 UTC
Starts to go downhill here:

      /* If these are the same operation types, we can associate them
         assuming no overflow.  */
      if (tcode == code)
        {
          bool overflow_p = false;
          bool overflow_mul_p;
          signop sign = TYPE_SIGN (ctype);
          wide_int mul = wi::mul (op1, c, sign, &overflow_mul_p);
          overflow_p = TREE_OVERFLOW (c) | TREE_OVERFLOW (op1);
          if (overflow_mul_p
              && ((sign == UNSIGNED && tcode != MULT_EXPR) || sign == SIGNED))
            overflow_p = true;
          if (!overflow_p)
            {
              mul = wide_int::from (mul, TYPE_PRECISION (ctype),
                                    TYPE_SIGN (TREE_TYPE (op1)));
              return fold_build2 (tcode, ctype, fold_convert (ctype, op0),
                                  wide_int_to_tree (ctype, mul));

which generates a too large constant.  op1 == 13, c == 536870912, the
multiplication result is sign-extended to unsigned long here (from int).

Note that "associate them assuming no overflow" sounds wrong here as the
operation does overflow int and this is what makes the result bogus.
We compute the multiplication in unsigned but then still sign-extend.

Sth. is fishy here.
Comment 5 Richard Biener 2016-03-21 11:45:25 UTC
4.9 had

      if (tcode == code)
        {
          double_int mul;
          bool overflow_p;
          unsigned prec = TYPE_PRECISION (ctype);
          bool uns = TYPE_UNSIGNED (ctype);
          double_int diop1 = tree_to_double_int (op1).ext (prec, uns);
          double_int dic = tree_to_double_int (c).ext (prec, uns);
          mul = diop1.mul_with_sign (dic, false, &overflow_p);
          overflow_p = ((!uns && overflow_p)
                        | TREE_OVERFLOW (c) | TREE_OVERFLOW (op1));
          if (!double_int_fits_to_tree_p (ctype, mul)
              && ((uns && tcode != MULT_EXPR) || !uns))
            overflow_p = 1;
          if (!overflow_p)
            return fold_build2 (tcode, ctype, fold_convert (ctype, op0),
                                double_int_to_tree (ctype, mul));

where double_int_to_tree does not do any kind of extension.  Given it
unconditonally uses an unsinged multiplication you can see it as a
zero-extension.
Comment 6 Richard Biener 2016-03-21 12:00:15 UTC
Testing the following - 4.9 performed a wide unsigned multiplication

@@ -6376,18 +6376,17 @@ extract_muldiv_1 (tree t, tree c, enum t
          bool overflow_p = false;
          bool overflow_mul_p;
          signop sign = TYPE_SIGN (ctype);
-         wide_int mul = wi::mul (op1, c, sign, &overflow_mul_p);
+         unsigned prec = TYPE_PRECISION (ctype);
+         wide_int mul = wi::mul (wide_int::from (op1, prec, sign),
+                                 wide_int::from (c, prec, sign),
+                                 UNSIGNED, &overflow_mul_p);
          overflow_p = TREE_OVERFLOW (c) | TREE_OVERFLOW (op1);
          if (overflow_mul_p
              && ((sign == UNSIGNED && tcode != MULT_EXPR) || sign == SIGNED))
            overflow_p = true;
          if (!overflow_p)
-           {
-             mul = wide_int::from (mul, TYPE_PRECISION (ctype),
-                                   TYPE_SIGN (TREE_TYPE (op1)));
-             return fold_build2 (tcode, ctype, fold_convert (ctype, op0),
-                                 wide_int_to_tree (ctype, mul));
-           }
+           return fold_build2 (tcode, ctype, fold_convert (ctype, op0),
+                               wide_int_to_tree (ctype, mul));
        }
 
       /* If these operations "cancel" each other, we have the main
Comment 7 Richard Biener 2016-03-21 15:14:33 UTC
Hmm.  FAILs gcc.dg/torture/20141202-1.c (at least).
Comment 8 Richard Biener 2016-03-22 13:23:32 UTC
Author: rguenth
Date: Tue Mar 22 13:23:00 2016
New Revision: 234401

URL: https://gcc.gnu.org/viewcvs?rev=234401&root=gcc&view=rev
Log:
2016-03-22  Richard Biener  <rguenther@suse.de>

	PR middle-end/70333
	* fold-const.c (extract_muldiv_1): Properly perform multiplication
	in the wide type.

	* gcc.dg/torture/pr70333.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr70333.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog
Comment 9 Richard Biener 2016-03-22 13:23:39 UTC
Fixed on trunk sofar.
Comment 10 Richard Biener 2016-04-06 07:45:46 UTC
Fixed.
Comment 11 Richard Biener 2016-04-06 07:46:06 UTC
Author: rguenth
Date: Wed Apr  6 07:45:34 2016
New Revision: 234772

URL: https://gcc.gnu.org/viewcvs?rev=234772&root=gcc&view=rev
Log:
2016-04-06  Richard Biener  <rguenther@suse.de>

	Backport from mainline
	2016-03-30  Richard Biener  <rguenther@suse.de>

	PR middle-end/70450
	* fold-const.c (extract_muldiv_1): Fix thinko in wide_int::from
	usage.

	* gcc.dg/torture/pr70450.c: New testcase.

	2016-03-22  Richard Biener  <rguenther@suse.de>

	PR middle-end/70333
	* fold-const.c (extract_muldiv_1): Properly perform multiplication
	in the wide type.

	* gcc.dg/torture/pr70333.c: New testcase.

	2016-04-04  Richard Biener  <rguenther@suse.de>

	PR rtl-optimization/70484
	* rtl.h (canon_output_dependence): Declare.
	* alias.c (canon_output_dependence): New function.
	* dse.c (record_store): Use canon_output_dependence rather
	than canon_true_dependence.

	* gcc.dg/torture/pr70484.c: New testcase.

	2016-03-31  Richard Biener  <rguenther@suse.de>

	PR c++/70430
	* typeck.c (cp_build_binary_op): Fix operand order of vector
	conditional in truth op handling.

	* g++.dg/ext/vector30.C: New testcase.

Added:
    branches/gcc-5-branch/gcc/testsuite/g++.dg/ext/vector30.C
    branches/gcc-5-branch/gcc/testsuite/gcc.dg/torture/pr70333.c
    branches/gcc-5-branch/gcc/testsuite/gcc.dg/torture/pr70450.c
    branches/gcc-5-branch/gcc/testsuite/gcc.dg/torture/pr70484.c
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/alias.c
    branches/gcc-5-branch/gcc/cp/ChangeLog
    branches/gcc-5-branch/gcc/cp/typeck.c
    branches/gcc-5-branch/gcc/dse.c
    branches/gcc-5-branch/gcc/fold-const.c
    branches/gcc-5-branch/gcc/rtl.h
    branches/gcc-5-branch/gcc/testsuite/ChangeLog