Re: [PATCH, PR38785] Throttle PRE at -O3

[Maxim Kuvyrkov]

On 27/04/2012, at 1:17 AM, Richard Guenther wrote:

> On Wed, Apr 25, 2012 at 8:06 AM, Maxim Kuvyrkov <ma...@codesourcery.com> 
> wrote:
> 
...
> +ppre_n_insert_for_speed_p (pre_expr expr, basic_block block,
> +                          unsigned int inserts_needed)
> +{
> +  /* The more expensive EXPR is, the more we should be prepared to insert
> +     in the predecessors of BLOCK to make EXPR fully redundant.
> +     For now, only recognize AND, OR, XOR, PLUS and MINUS of a multiple-use
> +     SSA_NAME with a constant as cheap.  */
> 
> the function implementation is odd.  cost is always 1 when used, and
> both memory loads and calls are always cheap, but for example casts are not?
> Isn't
> 
> return EDGE_COUNT (block->preds) * cost >= inserts_needed;
> 
> always true?  Or is inserts_needed not what it suggests?

It /almost/ is what it suggests.  To account for the fact that inserts will not 
benefit some of the paths (50% in the initial version of the patch, and a more 
precise estimate in a latter version) we tell/lie ppre_n_insert_for_speed_p 
that we need a greater number of the inserts (e.g., 2x the real number) that we 
will perform.

> 
> +             /* Insert only if we can remove a later expression on a path
> +                that we want to optimize for speed.
> +                The phi node that we will be inserting in BLOCK is not free,
> +                and inserting it for the sake of !optimize_for_speed 
> successor
> +                may cause regressions on the speed path.  */
> +             FOR_EACH_EDGE (succ, ei, block->succs)
> +               {
> +                 if (bitmap_set_contains_value (PA_IN (succ->dest), val))
> +                   {
> +                     if (optimize_edge_for_speed_p (succ))
> +                       do_insertion = true;
> +
> +                     ++pa_succs;
> +                   }
> +               }
> 
> the change up to here looks good to me - can you factor out the command-line
> switch plus this optimize_edge_for_speed_p test into a separate patch
> (hereby approved)?

Attached is what I checked in after retesting on a fresh mainline.  This part 
of the patch by itself turns out to fix the bitmnp01 regression in what seems 
to be a reliable way.  Therefore, I'll mark PR38785 as fixed.

Thank you,

--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics

pr38785-2.ChangeLog
Description: Binary data

pr38785-2.patch
Description: Binary data