16590 – [3.4 regression] Incorrect execution when compiling with -O2 (original) (raw)

Description Michele Galante 2004-07-16 14:56:33 UTC

I have reduced the testcase to a small file that only includes in order to show where the execution fails. The assert in main() should never fail, but it fails if you compile with:

g++ -O2 bug.cpp

The assert does not fail if you compile with one of the following: g++ -O0 bug.cpp g++ -O1 bug.cpp g++ -O2 -DWORKAROUND bug.cpp

This happens with gcc 3.4.1 (mingw special) on Windows XP and gcc 3.4.0 on SCO OPENSERVER 5.0.6, but NOT with gcc 3.4.0 on SUN Solaris; perhaps a bug in generating code for INTEL X86?

Output of "gcc -v" for gcc 3.4.1/MinGW on Windows XP (assert FAILS): Reading specs from c:/mingw/bin/../lib/gcc/mingw32/3.4.1/specs Configured with: ../gcc/configure --with-gcc --with-gnu-ld --with-gnu-as -- host=mingw32 --target=mingw32 --prefix=/mingw --enable-threads --disable-nls -- enable-languages=c,c++,f77,ada,objc,java --disable-win32-registry --disable- shared --enable-sjlj-exceptions --enable-libgcj --disable-java-awt --without- x --enable-java-gc=boehm --disable-libgcj-debug --enable-interpreter --enable- hash-synchronization --enable-libstdcxx-debug Thread model: win32 gcc version 3.4.1 (mingw special)

Output of "gcc -v" for gcc 3.4.0 on SCO OpenServer (assert FAILS): Reading specs from /home/local/gcc-3.4.0/bin/../lib/gcc/i686-pc- sco3.2v5.0.6/3.4.0/specs Configured with: ../gcc-3.4.0/configure --with-gnu-as --prefix=/usr/local/gcc- 3.4.0 Thread model: single gcc version 3.4.0

Output of "gcc -v" for gcc 3.4.0 on SUN Solaris (assert does NOT FAIL): Reading specs from /usr/local/gcc-3.4.0/lib/gcc/sparc-sun- solaris2.8/3.4.0/specs Configured with: ../gcc-3.4.0/configure --disable-shared -- prefix=/usr/local/gcc-3.4.0 Thread model: posix gcc version 3.4.0

// -------------------- bug.cpp ------------------------

#include

typedef char* iterator_type;

struct queue;

struct iterator { #ifdef WORKAROUND iterator_type volatile pos_; #else iterator_type pos_; #endif queue const* queue_;

iterator(queue const* queue, iterator_type pos) : pos_(pos), queue_(queue) {} iterator& operator--(); };

struct queue { iterator_type const first_, last_; iterator_type put_, get_;

queue(iterator_type first, iterator_type last) : first_(first), last_(last), put_(first), get_(first) {}

iterator pbegin() const { return iterator(this, put_); } iterator pend() const { iterator end(this, get_); return --end; } };

inline iterator& iterator::operator--() { if(pos_ == queue_->first_) pos_ = queue_->last_; --pos_; return *this; }

int main() { char mem[4+1]; queue buf(mem, mem+4+1); assert(buf.pend().pos_ == buf.pbegin().pos_+4); return 0; }

Comment 1 Wolfgang Bangerth 2004-07-16 15:39:46 UTC

Confirmed. Here's a very fragile testcase (if you change a bit, the bug goes away):

extern "C" void abort(); struct iterator { char * p; int *dummy; }; static iterator pend(char * start) { iterator p = {start, 0}; if (p.p == start) p.p = start+5; --p.p; return p; } int main() { char mem[4+1]; if(pend(mem).p != mem+4) abort (); }

g/x> /home/bangerth/bin/gcc-3.4-pre/bin/c++ x.cc ; ./a.out
g/x> /home/bangerth/bin/gcc-3.4-pre/bin/c++ -O2 x.cc ; ./a.out
Aborted

The bug doesn't happen with 3.3.4 and mainline, but who knows whether this is due to a fix or just by chance. It's a regression in any case, so should be investigated, then we can see whether the same fix is necessary on other branches than 3.4.x as well.

W.

Comment 2 Mark Mitchell 2004-08-17 23:02:51 UTC

This is a GCSE bug.

In the 07.addressof dump, we have:

(insn 56 104 57 1 (parallel [ (set (reg/f:SI 77) (plus:SI (reg/f:SI 20 frame) (const_int -11 [0xfffffff5]))) (clobber (reg:CC 17 flags)) ]) 138 {*addsi_1} (nil) (nil))

(insn 57 56 58 1 (set (mem/s:SI (plus:SI (reg/f:SI 20 frame) (const_int -24 [0xffffffe8])) [4 p.p+0 S4 A32]) (reg/f:SI 77)) 36 {*movsi_1} (nil) (nil))

(code_label 58 57 105 2 3 "" [1 uses])

(note 105 58 62 2 [bb 2] NOTE_INSN_BASIC_BLOCK)

(insn 62 105 63 2 (set (reg:SI 78 [ p.p ]) (mem/s:SI (plus:SI (reg/f:SI 20 frame) (const_int -24 [0xffffffe8])) [4 p.p+0 S4 A32])) 36 {*movsi_1} (nil) (nil))

(insn 63 62 64 2 (parallel [ (set (reg:SI 79) (plus:SI (reg:SI 78 [ p.p ]) (const_int -1 [0xffffffff]))) (clobber (reg:CC 17 flags)) ]) 138 {*addsi_1} (nil) (nil))

After GCSE, we have:

(insn 56 104 114 1 (parallel [ (set (reg/f:SI 77) (plus:SI (reg/f:SI 20 frame) (const_int -11 [0xfffffff5]))) (clobber (reg:CC 17 flags)) ]) 138 {*addsi_1} (nil) (nil))

(insn 114 56 58 1 (set (reg/f:SI 86 [ start ]) (reg/f:SI 77)) -1 (nil) (nil))

(code_label 58 114 105 2 3 "" [1 uses])

(note 105 58 112 2 [bb 2] NOTE_INSN_BASIC_BLOCK)

(insn 112 105 63 2 (set (reg/f:SI 78 [ p.p ]) (reg/f:SI 86 [ start ])) -1 (nil) (expr_list:REG_EQUAL (mem/s:SI (plus:SI (reg/f:SI 20 frame) (const_int -24 [0xffffffe8])) [4 p.p+0 S4 A32]) (nil)))

Note that we now have a REG_EQUAL note on insn 112 that is a lie, since insn 114 no longer sets the memory.

Comment 3 Mark Mitchell 2004-08-17 23:10:46 UTC

Created attachment 6951 [details] Patch

I see no justification for GCSE assuming that, after PRE eliminiation, the REG_EQUAL note is valid -- unless the value is a constant, and in that I don't think we should be travelling this code path.

This patch fixes the affected test-case by eliminating the note.

Comment 4 Mark Mitchell 2004-08-17 23:11:42 UTC

Jeff, would you please take a look at my analysis and confirm/deny?

Thanks,

-- Mark

Comment 5 Mark Mitchell 2004-08-29 19:02:48 UTC

Postponed until GCC 3.4.3.

Comment 8 Mark Mitchell 2004-08-29 19:42:09 UTC

Fixed in GCC 3.4.2.

Comment 9 Jeffrey A. Law 2004-08-30 20:22:47 UTC

Looks more like a bug in load/store motion to me, not a bogus REG_EQUAL note.

Comment 12 Jeffrey A. Law 2004-09-01 05:41:22 UTC

Fixed with today's checkin to gcse.c.

Comment 13 Volker Reichelt 2004-09-01 10:19:36 UTC

The testcase gcc/gcc/testsuite/g++.dg/opt/loop1.C is still XFAILed on mainline (which was wrong IMHO anyway) and 3.4 branch. This should be fixed before the 4.3.2 release.