On January 15, 2016 11:06:40 PM GMT+01:00, Bill Schmidt
<wschm...@linux.vnet.ibm.com> wrote:
>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?
OK.
Thanks,
Richard.
>Thanks,
>Bill
>
>
>2016-01-15 Bill Schmidt <wschm...@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++)
> {
