Bug 53663 - [4.7 Regression] inconsistent inline handling of bool within union
Summary: [4.7 Regression] inconsistent inline handling of bool within union
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.7.2
: P2 normal
Target Milestone: 4.7.3
Assignee: Richard Biener
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2012-06-13 22:46 UTC by brendan.jones.it
Modified: 2012-12-03 16:54 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.8.0
Known to fail: 4.7.2
Last reconfirmed: 2012-06-14 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description brendan.jones.it 2012-06-13 22:46:50 UTC
Please refer to the following:

/* -*- Mode: C ; c-basic-offset: 4 -*- */
/* gcc test_jack.c -Wall -Wextra -o test -O1 && ./test ; echo $?
   should print 0, prints 10
   gcc known to be affected:

Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --disable-build-with-cxx --disable-build-poststage1-with-cxx --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto --enable-plugin --enable-initfini-array --enable-java-awt=gtk --disable-dssi --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre --enable-libgcj-multifile --enable-java-maintainer-mode --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib --with-ppl --with-cloog --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 4.7.0 20120507 (Red Hat 4.7.0-5) (GCC) 
*/
#include <stdio.h>
union u
{
    int i = 0;
    _Bool b = 0;
};

void f(union u * vp, union u v)
{
    *vp = v;
}

int main()
{
    union u v = 0;
    union u v1 = 0;
    union u v2 = 0;

    v.i = 10;
    f(&v1, v);

    v.b = 0;
    f(&v2, v);
    if (v2.b) printf("True\n");
    else printf("False\n");
    return v2.b;
}
Comment 1 Andrew Pinski 2012-06-13 23:19:40 UTC
I bet this was already fixed in 4.7.1 though I have not tested it yet.
Comment 2 Richard Biener 2012-06-14 08:40:43 UTC
The testcase does not compile for me, it has errors:

> /space/rguenther/install/gcc-4.7.0/bin/gcc t.c -Wall -Wextra -o t -O1
t.c:4:11: error: expected ':', ',', ';', '}' or '__attribute__' before '=' token
t.c: In function 'main':
t.c:15:11: error: invalid initializer
t.c:16:11: error: invalid initializer
t.c:17:11: error: invalid initializer
t.c:19:6: error: 'union u' has no member named 'i'
t.c:22:6: error: 'union u' has no member named 'b'
t.c:24:11: error: 'union u' has no member named 'b'
t.c:26:14: error: 'union u' has no member named 'b'
t.c:27:1: warning: control reaches end of non-void function [-Wreturn-type]
Comment 3 brendan.jones.it 2012-06-14 09:00:37 UTC
Sorry - wrong file

/* -*- Mode: C ; c-basic-offset: 4 -*- */
/* gcc test.c -Wall -Wextra -o test -O1 && ./test ; echo $?
   should print 0, prints 10
   gcc known to be affected:

Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --disable-build-with-cxx --disable-build-poststage1-with-cxx --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto --enable-plugin --enable-initfini-array --enable-java-awt=gtk --disable-dssi --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre --enable-libgcj-multifile --enable-java-maintainer-mode --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib --with-ppl --with-cloog --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 4.7.0 20120507 (Red Hat 4.7.0-5) (GCC) 
*/

union u
{
    int i;
    _Bool b;
};

void f(union u * vp, union u v)
{
    *vp = v;
}

int main()
{
    union u v;
    union u v1;
    union u v2;

    v.i = 10;
    f(&v1, v);

    v.b = 0;
    f(&v2, v);
    return v2.b;
}
Comment 4 brendan.jones.it 2012-06-24 07:56:59 UTC
Hi,

have you been able to replicate this issue with the second example I have sent?

If so is there a known workaround that we can use?

many thanks

Brendan
Comment 5 Fryderyk Dziarmagowski 2012-09-23 10:01:02 UTC
The bug is WAITING for 3 months already. Could it be possible to verify it? The problem is affecting wide range of jack-audio users.
Comment 6 Mikael Pettersson 2012-09-23 11:56:50 UTC
I can reproduce the wrong-code with gcc 4.7-20120922 and 4.8-20120916, at -O1, on x86_64-linux. -O2 avoids the wrong-code, but if I replace the "*vp = v" with the equivalent __builtin_memcpy() then -O2 also generates wrong code. gcc-4.6 seems to work correctly.
Comment 7 Mikael Pettersson 2012-09-23 17:14:30 UTC
The regression started with Richard G's fix for PR38885 in r179556:
http://gcc.gnu.org/ml/gcc-cvs/2011-10/msg00150.html
http://gcc.gnu.org/ml/gcc-patches/2011-10/msg00326.html

It may be yet another SRA problem.
Comment 8 Mikael Pettersson 2012-09-23 18:44:53 UTC
(In reply to comment #7)
> It may be yet another SRA problem.

Perhaps not, -fno-tree-sra makes no difference.  Looking through the tree dumps I see the wrong code first appearing in the .fre1 dump.  Compiling with -O1 -fno-tree-fre does avoid the wrong code.
Comment 9 Fryderyk Dziarmagowski 2012-09-23 19:52:29 UTC
Confirmed, jackd compiled with -O1 -fno-tree-fre works as expected
Comment 10 Mikael Pettersson 2012-09-23 22:12:07 UTC
Although -fno-tree-fre works for the test case in #c3, adjusting it to use __builtin_memcpy() for the assignment in f() results in wrong code even with -fno-tree-fre.

The bug seems _Bool-specific. Replacing the _Bool with unsigned char b:1 gives working code regardless of compiler options.
Comment 11 Richard Biener 2012-09-24 09:08:17 UTC
Mine.
Comment 12 Jakub Jelinek 2012-09-24 09:14:31 UTC
Guess for BOOLEAN_TYPE in unions we can't look just at the single bit, but also all other bits of the boolean type, because we rely that the bool doesn't contain other values than false/true.
Comment 13 Richard Biener 2012-09-24 11:44:32 UTC
This boils down to the question whether reading a 1-bit precision quantity
from memory has to disregard the upper bits or not (I think we had similar
issues with SRA).  Thus, whether reading a _Bool from memory is a
bitfield extract or not (expansion does not treat it as bitfield extract
because the FIELD_DECLs size is 8, not 1).

We go into

  /* 3) Assignment from a constant.  We can use folds native encode/interpret
     routines to extract the assigned bits.  */

which has the issue that it doesn't work if TYPE_PRECISION is not equal
to TYPE_SIZE.  At least not for detecting redundant stores (it does work
for folding a read of v.b though).

I have a patch.
Comment 14 Richard Biener 2012-09-24 13:14:00 UTC
We still want to possibly optimize

extern void abort (void);

union u
{
  int i;
  _Bool b;
};

void f(union u * vp, union u v)
{
  *vp = v;
}

int main()
{
  union u v;
  union u v1;
  union u v2;

  v.i = 0;
  f(&v1, v);

  v.b = 0;
  f(&v2, v);
  if (v2.b != 0)
    abort ();
  return 0;
}

though we might be able to trigger TBAA issues when removing a store
that would merely change the effective type (without changing the
underlying value).  Of course we try hard (on the tree level) to
make DWIM code work, but still ... thus,

 <float> = 1.;
 <int> = 0;
 <float> = 0.;
 ... = <float>;

if we remove the store <float> = 0. as redundant (it stores a value
already there) then further optimizations might re-order the float
and the int store.  We don't perform redundant store elimination here
because 0. and 0 are not operand_equal_p though - but it works for shorts.

extern void abort (void);

union u
{
  int i;
  short f;
} v;

short foo (short *f)
{
  *f = 1;
  v.i = 0;
  v.f = 0;
  return *f;
}

int main()
{
  if (foo (&v.f) != 0)
    abort ();
  return 0;
}

(still doesn't break though).
Comment 15 Richard Biener 2012-09-25 07:52:01 UTC
Author: rguenth
Date: Tue Sep 25 07:51:51 2012
New Revision: 191694

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191694
Log:
2012-09-25  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/53663
	* tree-ssa-sccvn.c (vn_reference_lookup_3): Conditional
	native encode/interpret translation on VN_WALKREWRITE.

	* gcc.dg/torture/pr53663-1.c: New testcase.
	* gcc.dg/torture/pr53663-2.c: Likewise.
	* gcc.dg/torture/pr53663-3.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr53663-1.c
    trunk/gcc/testsuite/gcc.dg/torture/pr53663-2.c
    trunk/gcc/testsuite/gcc.dg/torture/pr53663-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-sccvn.c
Comment 16 Richard Biener 2012-09-25 07:55:30 UTC
Fixed on trunk sofar.
Comment 17 Fryderyk Dziarmagowski 2012-09-25 08:31:25 UTC
With above fix 4.7.2 works as expected too.
Comment 18 Richard Biener 2012-12-03 16:53:39 UTC
Author: rguenth
Date: Mon Dec  3 16:53:25 2012
New Revision: 194101

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=194101
Log:
2012-12-03  Richard Biener  <rguenther@suse.de>

	Backport from mainline
	2012-09-24  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/53663
	* tree-ssa-sccvn.c (vn_reference_lookup_3): Conditional
	native encode/interpret translation on VN_WALKREWRITE.

	* gcc.dg/torture/pr53663-1.c: New testcase.
	* gcc.dg/torture/pr53663-2.c: Likewise.
	* gcc.dg/torture/pr53663-3.c: Likewise.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr53663-1.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr53663-2.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr53663-3.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-ssa-sccvn.c
Comment 19 Richard Biener 2012-12-03 16:54:13 UTC
Fixed.