[PATCH][RFC] PR middle-end/22141 GIMPLE store widening pass

Kyrill Tkachov kyrylo.tkachov@foss.arm.com
Wed Aug 3 09:59:00 GMT 2016


Hi Richard,

On 18/07/16 13:22, Richard Biener wrote:

<snip>

> +      /* Record the original statements so that we can keep track of
> +        statements emitted in this pass and not re-process new
> +        statements.  */
> +      for (gsi = gsi_after_labels (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> +       {
> +         gimple *stmt = gsi_stmt (gsi);
> +         if (!is_gimple_debug (stmt))
> +           orig_stmts.add (stmt);
> +         num_statements++;
> +       }
>
> please use gimple_set_visited () instead, that should be cheaper.
>
>
> +      do
> +       {
> +         changes_made = false;
> +         for (gsi = gsi_after_labels (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> +           {
> ...
> +       }
> +      while (changes_made);
>
> looks pretty quadratic to me.  Instead of tracking things with m_curr_base_expr
> why not use a hash-map to track stores related to a base?

I've implemented this scheme but I'm having trouble making it work.
In particular I have a hash_map keyed on a 'tree' that is the base
object (as extracted by get_inner_reference) but I can't get the hash_map
to properly extract the already recorded stores to the same base.
For example for the simple code:
struct bar {
   int a;
   char b;
   char c;
   char d;
   char e;
   char f;
   char g;
};

void
foo1 (struct bar *p)
{
   p->b = 0;
   p->a = 0;
   p->c = 0;
   p->d = 0;
   p->e = 0;
}

As we can see, the stores are all to the same object and should
be recognised as such.

The base of the first store is recorded as:
<mem_ref 0x7f527a482820 ...>
and for the second store as <mem_ref 0x7f527a482848 ...>
where the dumps of the two mem_refs are identical except for that first
hex number (their address in memory?)
In my first version of the patch I compare these with operand_equal_p and that
detects that they are the same, but in the hash_map they are not detected
as equal. Is there some special hashing function I must specify?

Thanks,
Kyrill



More information about the Gcc-patches mailing list