[Bug middle-end/36149] -O2 optimization generates wrong code

[dino at concisoft dot com]

--- Comment #11 from dino at concisoft dot com  2008-06-06 21:13 ---
Created an attachment (id=15727)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=15727&action=view)
A relatively simple self contained piece of code that reproduces the problem.

Please see the comments in the Makefile for info on how to reproduce the
problem.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36149

[Bug c++/36149] -O2 optimization generates wrong code

[dino at concisoft dot com]

--- Comment #9 from dino at concisoft dot com  2008-05-07 06:15 ---
Understood. Just haven't been able to reproduce on a small piece of code :-(

It seems GNU C++ compiler doesn't give strict-aliasing warnings.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36149

[Bug c++/36149] -O2 optimization generates wrong code

[dino at concisoft dot com]

--- Comment #7 from dino at concisoft dot com  2008-05-06 16:54 ---
-O2 -fno-strict-aliasing does work.

I think the LC and LLC functions always cast from a derived class to a base
class. That shouldn't cause aliasing problems, but I need to learn more about
that.

Thanks.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36149

[Bug c++/36149] -O2 optimization generates wrong code

[dino at concisoft dot com]

--- Comment #5 from dino at concisoft dot com  2008-05-06 15:42 ---
Are we missing a const, or would you care to give a bit more specific comment?

I see that:

template static const LINK* LCC(Y* X) {
return static_cast(X);
}

could be missing a const in the declaration of Y* X, should probably be const
Y* X. Is that what you're referring to?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36149

[Bug c++/36149] -O2 optimization generates wrong code

[dino at concisoft dot com]

--- Comment #3 from dino at concisoft dot com  2008-05-06 14:18 ---
template static LINK* LC(Y* X) {
return static_cast(X);
}
template static const LINK* LCC(Y* X) {
return static_cast(X);
}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36149

[Bug c++/36149] -O2 optimization generates wrong code

[dino at concisoft dot com]

--- Comment #1 from dino at concisoft dot com  2008-05-06 07:19 ---
The same code works fine with -O1 and -O3.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36149

[Bug c++/36149] New: -O2 optimization generates wrong code

[dino at concisoft dot com]

GCC version:
gcc (GCC) 4.1.2 20070925 (Red Hat 4.1.2-27)

It is really hard to reproduce and goes away even with minimal code changes.

Happens in a unit test. 

Host is x86_64: "model name  : Intel(R) Core(TM)2 CPU  6300  @
1.86GHz" from /proc/cpuinfo.

The following snippets might be useful to someone perhaps:

C code:

void pRemove(CLASS *d)
{
  LC(d)->m_back->m_fwd = LC(d)->m_fwd;
  LC(d)->m_fwd->m_back = LC(d)->m_back;
}

void prepend(CLASS *a)
{
pRemove(a);
LC(a)->m_fwd = head.m_fwd;
head.m_fwd->m_back = LC(a);
LC(a)->m_back = &head;
head.m_fwd = LC(a);
}

MapList::add():
// ...
m_list.remove(removed);
if (fTop) {
  m_list.prepend(item);
} else {
// ...


Disassembly w/ comments:

 8053c7f:   80 7d 8f 00 cmpb   $0x0,0xff8f(%ebp)###
if (fTop)
 8053c83:   89 02   mov%eax,(%edx)
 8053c85:   8b 07   mov(%edi),%eax
 8053c87:   89 3f   mov%edi,(%edi)
 8053c89:   89 50 04mov%edx,0x4(%eax)
 8053c8c:   89 7f 04mov%edi,0x4(%edi)

The above few lines perform the remove(removed) call and the if test out of
order, which is OK. After that, variables look like this:
14: item.m_back = (Vapi::DoubleLink *) 0x81d4400
13: item.m_fwd = (Vapi::DoubleLink *) 0x81d4400
12: item = (DownloadQueue::DQ_Item *) 0x81d4400
11: this->m_list.head.m_back = (Vapi::DoubleLink *) 0xffc26b3c
10: this->m_list.head.m_fwd = (Vapi::DoubleLink *) 0xffc26b3c
9: &this->m_list.head = (Vapi::DoubleLink *) 0xffc26b3c
8: removed.m_back = (Vapi::DoubleLink *) 0x81d43b0
7: removed.m_fwd = (Vapi::DoubleLink *) 0x81d43b0
6: removed = (DownloadQueue::DQ_Item *) 0x81d43b0

The m_list.head is empty, item is on no list, and removed is on no list.

Then the code for prepend(item) is supposed to execute:

 8053c8f:   0f 84 bf 01 00 00   je 8053e54

 8053c95:   8b 55 0cmov0xc(%ebp),%edx   ###
edx = item
 8053c98:   8b 42 04mov0x4(%edx),%eax   ###
eax = item->m_back (in pRemove)
 8053c9b:   89 d3   mov%edx,%ebx###
ebx = item
 8053c9d:   8b 12   mov(%edx),%edx  ###
edx = item->m_fwd
 8053c9f:   89 10   mov%edx,(%eax)  ###
item->m_back->m_fwd = edx (item->m_fwd)
 8053ca1:   8b 13   mov(%ebx),%edx  ###
edx = item->m_fwd
 8053ca3:   89 0b   mov%ecx,(%ebx)  ###
item->m_fwd = %ecx (removed?!)
 8053ca5:   89 42 04mov%eax,0x4(%edx)   ###
item->m_fwd->m_back = item->m_back
 8053ca8:   8b 45 9cmov0xff9c(%ebp),%eax###
eax = head
 8053cab:   8b 55 98mov0xff98(%ebp),%edx###
??
 8053cae:   89 59 04mov%ebx,0x4(%ecx)   ###
removed->m_back = item (?!)
 8053cb1:   89 43 04mov%eax,0x4(%ebx)   ###
item->m_back = head
 8053cb4:   89 5a 04mov%ebx,0x4(%edx)   ###
?? head->m_back = item
 8053cb7:   8b 5f 20mov0x20(%edi),%ebx

But that doesn't look all that great. The compiler somehow tries to interleave
the pRemove and the rest of prepend, but the result is pretty bad:

14: item.m_back = (Vapi::DoubleLink *) 0xffc26b3c
13: item.m_fwd = (Vapi::DoubleLink *) 0x81d43b0
12: item = (DownloadQueue::DQ_Item *) 0x81d4400
11: this->m_list.head.m_back = (Vapi::DoubleLink *) 0xffc26b3c
10: this->m_list.head.m_fwd = (Vapi::DoubleLink *) 0x81d4400
9: &this->m_list.head = (Vapi::DoubleLink *) 0xffc26b3c
8: removed.m_back = (Vapi::DoubleLink *) 0x81d4400
7: removed.m_fwd = (Vapi::DoubleLink *) 0x81d43b0
6: removed = (DownloadQueue::DQ_Item *) 0x81d43b0

Item and removed point to each other, with one of their pointers, m_list.head
points to item but not with both pointers, item points to head with one of its
pointers...

May not be able to provide full source that reproduces the problem (couldn't
generate a short piece of code that reproduces the problem.) But, could perhaps
provide additional info.


-- 
   Summary: -O2 optimization generates wrong code
   Product: gcc
   Version: 4.1.2
Status: UNCONFIRMED
  Severity: major
  Priority: P3
         Component: c++
AssignedTo: unassigned at gcc dot gnu dot org
ReportedBy: dino at concisoft dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36149