This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix PR68799


Hi,

In straight-line strength reduction, it can sometimes happen that more
than one conditional candidate can be predicated upon a PHI node P.
During processing of the first conditional candidate, a new PHI node may
be introduced in the same block as P, with new computations introduced
in predecessors of that block.  If this requires splitting one or more
edges, the PHI statement will change, and as a result it will now have a
different address in storage than it did before.

A problem arose because we cached the addresses of the PHI statements in
a mapping from statements to strength-reduction candidates.  If, after a
PHI statement changes, we refer to it by its new address when consulting
this mapping, we won't find the associated candidate.

Now, this shouldn't happen, because after creating the candidate table,
we should always refer to PHIs by the original address at the time of
the table's creation.  However, I had some sloppy code that caused us to
look up the PHI statement by its result operand, even though we already
had a handle to the original cached PHI statement.  When that code is
used, it can find the moved statement instead, and things go wrong.

This patch solves the problem by simply replacing the sloppy code with a
direct lookup based on the cached PHI statement address, so everything
remains consistent.

I haven't added a test case because this is a pretty difficult scenario
to re-create reliably.  The patch is pretty obvious, so I'm not too
concerned about that.

The problem was found originally by Matthias Klose while doing a PGO/LTO
build of python-2.7, and reported as PR68799.  Matthias has tested the
patch for me in his environment, and indeed it fixes the problem.  I've
bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions for GCC 5.3.  Currently doing the same for trunk.

Assuming no regressions is this ok for mainline, and thereafter for GCC
5 and GCC 4.9?

Thanks,
Bill


2016-01-15  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/68799
	* gimple-ssa-strength-reduction.c (create_phi_basis): Directly
	look up phi candidates in the statement-candidate map.
	(phi_add_costs): Likewise.
	(record_phi_increments): Likewise.
	(phi_incr_cost): Likewise.
	(ncd_with_phi): Likewise.
	(all_phi_incrs_profitable): Likewise.


Index: gcc/gimple-ssa-strength-reduction.c
===================================================================
--- gcc/gimple-ssa-strength-reduction.c	(revision 232394)
+++ gcc/gimple-ssa-strength-reduction.c	(working copy)
@@ -2267,7 +2267,7 @@ create_phi_basis (slsr_cand_t c, gimple from_phi,
   slsr_cand_t basis = lookup_cand (c->basis);
   int nargs = gimple_phi_num_args (from_phi);
   basic_block phi_bb = gimple_bb (from_phi);
-  slsr_cand_t phi_cand = base_cand_from_table (gimple_phi_result (from_phi));
+  slsr_cand_t phi_cand = *stmt_cand_map->get (from_phi);
   phi_args.create (nargs);
 
   /* Process each argument of the existing phi that represents
@@ -2376,7 +2376,7 @@ phi_add_costs (gimple phi, slsr_cand_t c, int one_
 {
   unsigned i;
   int cost = 0;
-  slsr_cand_t phi_cand = base_cand_from_table (gimple_phi_result (phi));
+  slsr_cand_t phi_cand = *stmt_cand_map->get (phi);
 
   /* If we work our way back to a phi that isn't dominated by the hidden
      basis, this isn't a candidate for replacement.  Indicate this by
@@ -2587,7 +2587,7 @@ static void
 record_phi_increments (slsr_cand_t basis, gimple phi)
 {
   unsigned i;
-  slsr_cand_t phi_cand = base_cand_from_table (gimple_phi_result (phi));
+  slsr_cand_t phi_cand = *stmt_cand_map->get (phi);
   
   for (i = 0; i < gimple_phi_num_args (phi); i++)
     {
@@ -2658,7 +2658,7 @@ phi_incr_cost (slsr_cand_t c, const widest_int &in
   unsigned i;
   int cost = 0;
   slsr_cand_t basis = lookup_cand (c->basis);
-  slsr_cand_t phi_cand = base_cand_from_table (gimple_phi_result (phi));
+  slsr_cand_t phi_cand = *stmt_cand_map->get (phi);
 
   for (i = 0; i < gimple_phi_num_args (phi); i++)
     {
@@ -3002,7 +3002,7 @@ ncd_with_phi (slsr_cand_t c, const widest_int &inc
 {
   unsigned i;
   slsr_cand_t basis = lookup_cand (c->basis);
-  slsr_cand_t phi_cand = base_cand_from_table (gimple_phi_result (phi));
+  slsr_cand_t phi_cand = *stmt_cand_map->get (phi);
 
   for (i = 0; i < gimple_phi_num_args (phi); i++)
     {
@@ -3212,7 +3212,7 @@ all_phi_incrs_profitable (slsr_cand_t c, gimple ph
 {
   unsigned i;
   slsr_cand_t basis = lookup_cand (c->basis);
-  slsr_cand_t phi_cand = base_cand_from_table (gimple_phi_result (phi));
+  slsr_cand_t phi_cand = *stmt_cand_map->get (phi);
 
   for (i = 0; i < gimple_phi_num_args (phi); i++)
     {



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]