Bug 78726 - [6 Regression] Incorrect unsigned arithmetic optimization
Summary: [6 Regression] Incorrect unsigned arithmetic optimization
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 7.0
: P2 normal
Target Milestone: 7.0
Assignee: Jakub Jelinek
URL:
Keywords: wrong-code
Depends on:
Blocks: yarpgen
  Show dependency treegraph
 
Reported: 2016-12-08 02:27 UTC by Dmitry Babokin
Modified: 2021-11-01 23:07 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-12-08 00:00:00


Attachments
reproducer (344 bytes, application/x-gzip)
2016-12-08 02:27 UTC, Dmitry Babokin
Details
gcc7-pr78726.patch (1.93 KB, patch)
2016-12-08 15:14 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Babokin 2016-12-08 02:27:14 UTC
Created attachment 40279 [details]
reproducer

-O2 produces different result than -O0 for unsigned arithmetic.

Reproduces with gcc 5, 6, and 7, while 4.9 works fine.

> cat driver.cpp
#include <stdio.h>

unsigned char A = 36;
unsigned char B = 173;
unsigned long int C;

extern void foo ();

int main () {
    foo ();
    printf("%lu\n", C);
    return 0;
}
> cat func.cpp
extern unsigned char A;
extern unsigned char B;
extern unsigned long int C;

void foo() {
  unsigned a = ~A;
  C = a * B * B + 1023094746 * a;
}

> g++ -O0 -o no_opt driver.cpp func.cpp
> ./no_opt
799092689
> g++ -O2 -o opt driver.cpp func.cpp
> ./opt
800200062
Comment 1 Jakub Jelinek 2016-12-08 10:01:41 UTC
Single file testcase:
unsigned char b = 36, c = 173;
unsigned int d;

__attribute__((noinline, noclone)) void
foo (void)
{
  unsigned a = ~b;
  d = a * c * c + 1023094746 * a;
}

int
main ()
{
  if (__SIZEOF_INT__ != 4 || __CHAR_BIT__ != 8)
    return 0;
  asm volatile ("" : : "g" (&b), "g" (&c) : "memory");
  foo ();
  if (d != 799092689)
    __builtin_abort ();
  return 0;
}
Comment 2 Jakub Jelinek 2016-12-08 10:05:21 UTC
Started with r220164, let me have a look.
Comment 3 Jakub Jelinek 2016-12-08 10:27:37 UTC
Most likely one of the endless reassoc bugs not properly updating or invalidating range information.  Before reassoc1 we have:
  # RANGE [4294967040, 4294967295]
  a_11 = (unsigned int) _3;
  c.1_4 = c;
  # RANGE [0, 255] NONZERO 255
  _5 = (unsigned int) c.1_4;
  # RANGE ~[1, 4294902015]
  _6 = _5 * a_11;
  # RANGE ~[1, 4278320895]
  _7 = _5 * _6;
which looks correct, a_11 is int [-256, 1] converted to unsigned int, and c is unsigned char.
But reassoc1 turns this into:
  # RANGE [4294967040, 4294967295]
  a_11 = (unsigned int) _3;
  c.1_4 = c;
  # RANGE [0, 255] NONZERO 255
  _5 = (unsigned int) c.1_4;
  # RANGE ~[1, 4278320895]
  _7 = _5 * _5;
  _13 = _7 + 1023094746;
  _14 = _13 * a_11;
It should have reused the SSA_NAME (_7) for something different, _5 * _5 has a range of [0, 65025] and could have been used in debug stmts later on.
Comment 4 Jakub Jelinek 2016-12-08 15:14:32 UTC
Created attachment 40281 [details]
gcc7-pr78726.patch

Untested fix.
Comment 5 Jakub Jelinek 2016-12-09 09:22:08 UTC
Author: jakub
Date: Fri Dec  9 09:21:36 2016
New Revision: 243476

URL: https://gcc.gnu.org/viewcvs?rev=243476&root=gcc&view=rev
Log:
	PR tree-optimization/78726
	* tree-ssa-reassoc.c (make_new_ssa_for_def): Add OPCODE and OP
	argument.  For lhs uses in debug stmts, don't replace lhs with
	new_lhs, but with a debug temp set to new_lhs opcode op.
	(make_new_ssa_for_all_defs): Add OPCODE argument, pass OPCODE and
	OP down to make_new_ssa_for_def.
	(zero_one_operation): Call make_new_ssa_for_all_defs even when
	stmts_to_fix is empty, if *def has not changed yet.  Pass
	OPCODE to make_new_ssa_for_all_defs.

	* gcc.c-torture/execute/pr78726.c: New test.
	* gcc.dg/guality/pr78726.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr78726.c
    trunk/gcc/testsuite/gcc.dg/guality/pr78726.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-reassoc.c
Comment 6 Jakub Jelinek 2016-12-09 09:23:23 UTC
Fixed on the trunk so far.
Comment 7 Jakub Jelinek 2017-10-10 13:26:48 UTC
GCC 5 branch is being closed
Comment 8 Jakub Jelinek 2018-10-26 11:33:21 UTC
GCC 6 branch is being closed, fixed in 7.x.