Bug 39390 - [4.4 Regression] Bogus aliasing warning with std::set
Summary: [4.4 Regression] Bogus aliasing warning with std::set
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.4.0
: P2 normal
Target Milestone: 4.4.7
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, missed-optimization
: 42077 42087 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-03-06 13:06 UTC by Volker Reichelt
Modified: 2012-03-13 13:11 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-03-06 13:36:52


Attachments
Partially reduced testcase (1.16 KB, text/plain)
2009-03-06 13:10 UTC, Volker Reichelt
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Volker Reichelt 2009-03-06 13:06:18 UTC
Another problem with aliasing: The following code snippet triggers a bogus
warning when compiled on trunk with "-O -Wall":

==========================================================================
#include<set>

struct A
{
  A() : i() {}
  int i;
};

struct B
{
  bool operator() (const A& x, const A& y) const { return x.i < y.i; }
};

void foo()
{
  std::set<A, B> s;
  s.insert(A());
}
==========================================================================

bug.cc: In function 'void foo()':
bug.cc:11: warning: dereferencing pointer '__x.15' does break strict-aliasing rules
.../gcc-4.4-20090305/include/c++/4.4.0/bits/stl_tree.h:530: note: initialized from here
bug.cc:11: warning: dereferencing pointer '__x.15' does break strict-aliasing rules
.../gcc-4.4-20090305/include/c++/4.4.0/bits/stl_tree.h:530: note: initialized from here

This probably results in wrong code generation.
Comment 1 Volker Reichelt 2009-03-06 13:10:30 UTC
Created attachment 17405 [details]
Partially reduced testcase

Somewhat reduced testcase.
Comment 2 Volker Reichelt 2009-03-06 13:27:04 UTC
Oops, typo: I meant the bug is triggered with "-O3 -Wall".

The reduced testcase already triggers the warning with "-O2 -Wall" thanks to some additional "inline"s.
Comment 3 Richard Biener 2009-03-06 13:36:52 UTC
Confirmed with the reduced testcase only, at -O2 -Wall.  The warning happens
after PTA triggered by SRA.

Relevant constraints:

__y_7 = &s.64+64
D.2839_21 = __y_7
__x.8_43 = D.2839_21

and code:

  D.2839_21 = &__y_7->D.2545;
  D.2832_22 ={v} &0B->D.2545;
  if (D.2832_22 != 0B)
    goto <bb 6>;
  else
    goto <bb 4>;

<bb 4>:
  D.2888_42 = &__y_7->D.2545;
  if (D.2839_21 == D.2888_42)
    goto <bb 6>;
  else
    goto <bb 5>;

<bb 5>:
  __x.8_43 = (const struct _Rb_tree_node *) D.2839_21;
  D.2878_45 = __x.8_43->_M_value_field;

which shows that the warning is for dead code.  Thus this is not a
wrong-code problem.

VRP could figure out the equivalencies but doesn't.  The next DOM
pass figures out the second but not the first non-equivalency - that
survives until the next forwprop pass.

In the end the problem is that VRP doesn't do a good job cleaning up
after itself (there are single-argument PHIs left after it as well as
non-simplified comparisons):

<bb 3>:
Invalid sum of incoming frequencies 900, should be 10000
  # __y_8 = PHI <0B(2)>
  # __y_18 = PHI <__y_7(2)>
...

<bb 4>:
Invalid sum of incoming frequencies 3900, should be 102
  D.2837 = 1;
  D.2839_21 = &__y_18->D.2545;
  D.2832_22 ={v} &0B->D.2545;
  if (D.2832_22 != 0B)
    goto <bb 8>;
  else
    goto <bb 5>;

<bb 5>:
  D.2888_42 = &__y_7->D.2545;
  if (D.2839_21 == D.2888_42)
    goto <bb 8>;
  else
    goto <bb 6>;
Comment 4 Richard Biener 2009-03-06 14:16:20 UTC
Scheduling another pass_phi_only_cprop after VRP removes the single-argument
PHI nodes (I think that really cfg-cleanup should do this, as they usually
result from edge removal).  While this is reasonably cheap it doesn't get
rid of the conditionals but that would require a forwprop run which is not
that cheap (it requires only the forward_propagate_into_gimple_cond () pieces,
but has to run after removing single-argument PHIs, thus cannot run at
VRP substitution time).

Thus, the following would fix it:

Index: passes.c
===================================================================
--- passes.c    (revision 144665)
+++ passes.c    (working copy)
@@ -611,6 +611,8 @@ init_optimization_passes (void)
       NEXT_PASS (pass_copy_prop);
       NEXT_PASS (pass_merge_phi);
       NEXT_PASS (pass_vrp);
+      NEXT_PASS (pass_phi_only_cprop);
+      NEXT_PASS (pass_forwprop);
       NEXT_PASS (pass_dce);
       NEXT_PASS (pass_cselim);
       NEXT_PASS (pass_tree_ifcombine);

alternatively only adding pass_phi_only_cprop and calling
forward_propagate_into_gimple_cond from pass_tree_ifcombine.
Comment 5 Richard Biener 2009-03-08 15:42:12 UTC
Only the diagnostic part is a regression.
Comment 6 Andrew Pinski 2009-10-05 20:16:06 UTC
This is fixed on the trunk
Comment 7 rguenther@suse.de 2009-10-06 09:01:28 UTC
Subject: Re:  [4.4 Regression] Bogus aliasing
 warning with std::set

On Mon, 5 Oct 2009, pinskia at gcc dot gnu dot org wrote:

> ------- Comment #6 from pinskia at gcc dot gnu dot org  2009-10-05 20:16 -------
> This is fixed on the trunk

Indeed - the warning code was removed ;)
Comment 8 Paolo Carlini 2009-10-06 09:23:32 UTC
Then it's easy to fix in 4_4 too ;)
Comment 9 Paolo Carlini 2009-11-17 11:09:22 UTC
*** Bug 42077 has been marked as a duplicate of this bug. ***
Comment 10 Ryan Johnson 2009-11-17 11:16:49 UTC
(In reply to comment #3)
> the warning is for dead code.  Thus this is not a
> wrong-code problem.

Just to verify, does this (and comment #7) mean that the warning is harmless and can be ignored?
Comment 11 Paolo Carlini 2009-11-17 15:15:05 UTC
Yes.
Comment 12 Andrew Pinski 2009-11-18 04:02:44 UTC
*** Bug 42087 has been marked as a duplicate of this bug. ***
Comment 13 Jakub Jelinek 2012-03-13 13:11:24 UTC
Fixed in 4.5+, 4.4 is no longer supported.