[Bug libstdc++/43075] New: [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[Bug libstdc++/43075] New: [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[rguenth at gcc dot gnu dot org]

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


-- 
           Summary: [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rguenth at gcc dot gnu dot org


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

[Bug libstdc++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.5.0


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

[Bug libstdc++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[paolo dot carlini at oracle dot com]

------- Comment #1 from paolo dot carlini at oracle dot com  2010-02-15 11:44 -------
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?


-- 

paolo dot carlini at oracle dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jwakely dot gcc at gmail dot
                   |                            |com


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

[Bug libstdc++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2010-02-15 12:19 -------
(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).


-- 


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

[Bug libstdc++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[paolo dot carlini at oracle dot com]

------- Comment #3 from paolo dot carlini at oracle dot com  2010-02-15 12:27 -------
> 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.


-- 


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

[Bug libstdc++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2010-02-15 13:16 -------
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>;


-- 


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

[Bug libstdc++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[paolo dot carlini at oracle dot com]

------- Comment #5 from paolo dot carlini at oracle dot com  2010-02-15 14:02 -------
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.


-- 


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

[Bug libstdc++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2010-02-15 14:15 -------
  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.


-- 


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

[Bug libstdc++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[paolo dot carlini at oracle dot com]

------- Comment #7 from paolo dot carlini at oracle dot com  2010-02-15 14:21 -------
For sure Inc doesn't have any non-static data members...


-- 


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

[Bug libstdc++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs at -O0

[rguenth at gcc dot gnu dot org]

------- Comment #8 from rguenth at gcc dot gnu dot org  2010-02-15 14:53 -------
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.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rguenth at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-02-15 14:53:16
               date|                            |


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

[Bug c++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs

[rguenth at gcc dot gnu dot org]

------- Comment #9 from rguenth at gcc dot gnu dot org  2010-02-16 10:22 -------
FE issue.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libstdc++                   |c++
           Keywords|                            |wrong-code
            Summary|[4.5 Regression]            |[4.5 Regression]
                   |20_util/bind/ref2.cc FAILs  |20_util/bind/ref2.cc FAILs
                   |at -O0                      |


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

[Bug c++/43075] 20_util/bind/ref2.cc FAILs

[mmitchel at gcc dot gnu dot org]

------- Comment #10 from mmitchel at gcc dot gnu dot org  2010-02-17 17:51 -------
IIUC, this is not a regression.


-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4
            Summary|[4.5 Regression]            |20_util/bind/ref2.cc FAILs
                   |20_util/bind/ref2.cc FAILs  |


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

[Bug c++/43075] 20_util/bind/ref2.cc FAILs

[paolo dot carlini at oracle dot com]

------- Comment #11 from paolo dot carlini at oracle dot com  2010-02-17 17:57 -------
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... ;)


-- 

paolo dot carlini at oracle dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P4                          |P3


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

[Bug c++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs

[rguenth at gcc dot gnu dot org]

------- Comment #12 from rguenth at gcc dot gnu dot org  2010-02-17 21:32 -------
(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.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
            Summary|20_util/bind/ref2.cc FAILs  |[4.5 Regression]
                   |                            |20_util/bind/ref2.cc FAILs


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

[Bug c++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs

[jason at gcc dot gnu dot org]

------- Comment #13 from jason at gcc dot gnu dot org  2010-02-17 22:39 -------
Taking.


-- 

jason at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|rguenth at gcc dot gnu dot  |jason at gcc dot gnu dot org
                   |org                         |


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

[Bug c++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs

[jason at gcc dot gnu dot org]

------- Comment #14 from jason at gcc dot gnu dot org  2010-02-17 22:52 -------
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


-- 


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

[Bug c++/43075] [4.5 Regression] 20_util/bind/ref2.cc FAILs

[jason at gcc dot gnu dot org]

------- Comment #15 from jason at gcc dot gnu dot org  2010-02-17 22:58 -------
Fixed.


-- 

jason at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


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