make check-target-libstdc++-v3 RUNTESTFLAGS="--target_board=unix/-O0 conformance.exp=20_util/bind/ref2.cc" FAIL: 20_util/bind/ref2.cc execution test === libstdc++ Summary === # of expected passes 1 # of unexpected failures 1
If you really think this is a purely libstdc++ issue we gonna need *a lot* of help from the compiler people. Also consider that Jonathan, the author of the recent improvements to std::bind, will be in vacations for 2 weeks. Anyway, how can this be a [4.5 Regression] if the testcase didn't exist in 4.4 and, more specifically, uses C++0x features which do not make sense together with the 4.4 std::bind, which is just was the std::tr1::bind in the std:: namespace?
(In reply to comment #1) > If you really think this is a purely libstdc++ issue we gonna need *a lot* of > help from the compiler people. Also consider that Jonathan, the author of the > recent improvements to std::bind, will be in vacations for 2 weeks. Well, usually FAILs at -O0 but not -O2 hint at lifetime problems such as references to local vars being returned. Note that a patch of mine exposes this issue at -O2 ... so it's blocked by this issue. > Anyway, how can this be a [4.5 Regression] if the testcase didn't exist in 4.4 > and, more specifically, uses C++0x features which do not make sense together > with the 4.4 std::bind, which is just was the std::tr1::bind in the std:: > namespace? So the testcase is bogus? Then please remove it. I marked it as a regression prematurely because the patch I really want to apply will expose it at -O2 - which then makes it a regression against an earlier revision of trunk. And I'm quite lost in the myriads of variadic templates when trying to figure out what is going wrong (I tried for several hours already).
> So the testcase is bogus? Then please remove it. Nobody said is bogus. I said it didn't exist in 4.4, thus can't be a regression. Maybe we should xfail it, if we cannot understand in time what's going on.
It also fails with -O1 and -O2 -fno-strict-aliasing and with -O2 -fno-inline. Assembler differences for -O2 vs. -O2 with my patch (which effectively makes us see more must-aliases, thus "accept" slightly invalid strict-aliasing violating code): --- ref2.s.good 2010-02-15 13:47:03.000000000 +0100 +++ ref2.s.bad 2010-02-15 13:46:34.000000000 +0100 @@ -90,15 +90,16 @@ .cfi_startproc subq $40, %rsp .cfi_def_cfa_offset 48 - leaq 28(%rsp), %rsi + leaq 28(%rsp), %rdx movl $1, 28(%rsp) movq $_ZNK3Inc1fERi, (%rsp) movq $0, 8(%rsp) leaq 16(%rsp), %rdi - movq %rsi, 16(%rsp) + movq %rdx, 16(%rsp) movb %al, 16(%rsp) movl $_ZNK3Inc1fERi, %eax testb $1, %al + movq 16(%rsp), %rsi je .L10 movq _ZNK3Inc1fERi-1(%rsi), %rax .L10: that's _Z6test02v. You can see the aliasing byte-store to 16(%rsp) and the probably seemingly redundant load from 16(%rsp) that we maybe remove with strict-aliasing on. The tree code for the above is at .optimize time: <bb 2>: counter = 1; D.29754 = {}; __bound_args#0 = D.29754; D.25693._M_f.__pmf.__pfn = f; D.25693._M_f.__pmf.__delta = 0; D.25693._M_bound_args.D.25507.D.25257.D.25050._M_head_impl._M_data = &counter; this.24_34 = (struct Inc *) &D.25693._M_bound_args.D.25507; *this.24_34 = __bound_args#0; D.29831_51 = D.25693._M_bound_args.D.25507.D.25257.D.25050._M_head_impl._M_data; iftmp.27_55 = D.25693._M_f.__pmf.__pfn; D.29837_56 = (long int) iftmp.27_55; D.29838_57 = D.29837_56 & 1; if (D.29838_57 != 0) goto <bb 3>; else goto <bb 4>;
What I see right now is that Inc::f is called but &i != &counter. Instead, in the first test, that at line #53 of ref2.cc, Inc::operator() is called with &i == &counter, and everything is fine. I also tried moving out of line the member functions at lines #540 and #570 of <functional>, and also Int::f, and nothing changes at -O2, still doesn't fail, thus it's the inlining of something earlier in the call chain which makes a difference, and where we have to chase the temporary, it looks like.
D.25693._M_bound_args.D.25507.D.25257.D.25050._M_head_impl._M_data = &counter; this.24_34 = (struct Inc *) &D.25693._M_bound_args.D.25507; *this.24_34 = __bound_args#0; D.29831_51 = D.25693._M_bound_args.D.25507.D.25257.D.25050._M_head_impl._M_data; Thus with my patch we no longer CSE the load from _M_data with &counter but instead load it again because the store to D.25693._M_bound_args.D.25507 aliases it. Note that appearantly struct Inc D.29754 has zero size (and is packed to overlap with _M_data), and we expand it like ;; D.29754 = {}; (insn 6 5 0 /abuild/rguenther/trunk-g/x86_64-unknown-linux-gnu/libstdc++-v3/include/functional:1377 (clobber (reg:QI 77 [ D.29754 ])) -1 (nil)) ;; __bound_args#0 = D.29754; (insn 7 6 0 /abuild/rguenther/trunk-g/x86_64-unknown-linux-gnu/libstdc++-v3/include/functional:1377 (set (reg/v:QI 78 [ __bound_args#0 ]) (reg:QI 77 [ D.29754 ])) -1 (nil)) but the copy and later the indirect store transfers 1 byte of garbage.
For sure Inc doesn't have any non-static data members...
The issue _seems_ to be that the indirect assignment ;; *this.24_34 = __bound_args#0; is from ;; Function std::_Head_base<_Idx, _Head, true>::_Head_base(_UHead&&) [with _UHead = Inc, long unsigned int _Idx = 0ul, _Head = Inc] (null) ;; enabled by -tree-original { <<cleanup_point <<< Unknown tree: expr_stmt (void) (*(struct Inc *) this = *(const struct Inc &) (const struct Inc *) std::forward<Inc> ((struct type &) (struct Inc *) __h)) >>> >>; } where we do not see its zero-sizeness(?) and thus end up not removing the zero-sized assignment during cp_genericize_r. Bah, because it's an INIT_EXPR, not a MODIFY_EXPR. I have a patch.
FE issue.
IIUC, this is not a regression.
Mark, with your permission, I'm restoring P3: indeed, strictly speaking the failure of that specific testcase isn't a regression, because it didn't exist in gcc4.4.x, but I understand it uncovered a rather subtle problem in the C++ front-end. Anyway, I agree that we should figure out a plain C++ testcase, outside the libstdc++ testsuite, which fails only in 4.5, otherwise, I will understand if you release managers eventually decide to xfail the library testcase. But Jason is on it already... ;)
(In reply to comment #11) > Mark, with your permission, I'm restoring P3: indeed, strictly speaking the > failure of that specific testcase isn't a regression, because it didn't exist > in gcc4.4.x, but I understand it uncovered a rather subtle problem in the C++ > front-end. Anyway, I agree that we should figure out a plain C++ testcase, > outside the libstdc++ testsuite, which fails only in 4.5, otherwise, I will > understand if you release managers eventually decide to xfail the library > testcase. But Jason is on it already... ;) The C++ FE issue is a regression (we removed the expr_size langhook) that causes wrong-code.
Taking.
Subject: Bug 43075 Author: jason Date: Wed Feb 17 22:51:51 2010 New Revision: 156842 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156842 Log: PR c++/43075 * call.c (build_over_call): Don't create zero-sized assignments. * cp-gimplify.c (cp_genericize_r): Don't remove them here. * cp-objcp-common.c (cp_expr_size): Remove. * cp-tree.h: Remove prototype. Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/call.c trunk/gcc/cp/cp-gimplify.c trunk/gcc/cp/cp-objcp-common.c trunk/gcc/cp/cp-tree.h
Fixed.