Re: [PATCH] Further insn-attrtab.c speedup

[Michael Matz <matz at suse dot de>]

Hi,

On Fri, 18 Jun 2010, Jakub Jelinek wrote:

> This patch instead emits
>   enum attr_memory cached_memory ATTRIBUTE_UNUSED;
> ...
>     case 1685:  /* *vec_extractv2di_1_rex64 */
>     case 1684:  /* *vec_extractv2di_1_rex64_avx */
>       extract_constrain_insn_cached (insn);
>       if (!((1 << which_alternative) & 0x7))
>         {
>           return 1;
>         }
>       else if ((cached_memory = get_attr_memory (insn)) == MEMORY_BOTH)
>         {
>           return 3;
>         }
>       else if (cached_memory == MEMORY_LOAD)
> 
> Similar for conditions like if (something && (((get_attr_memory (insn) 
> == MEMORY_BOTH) || (get_attr_memory (insn) == MEMORY_LOAD)))) it emits 
> if (something && ((((cached_memory = get_attr_memory (insn)) == 
> MEMORY_BOTH || (cached_memory == MEMORY_LOAD)))) The resulting 
> insn-attrtab.c compiled with -Wuninitialized, so I hope I've done all 
> the conditions when the cached attribute can be used finally right.

There's quite some complexity you had to add to handle such situations 
correctly:

  if (cond1 && get_attr_x () ...)
    ...
  else if (cond2 && get_attr_x () ...)

Here the first get_attr_x call doesn't dominate the second one, hence you 
can't transform this sequence into just one call using the cached value.  
But backends must be able to determine all attributes for all 
instructions, so we _can_ transform this into

  cached_x = get_attr_x ();
  if (cond1 && cached_x ...)
    ...
  else if (cond2 && cached_x ...)

Which is what my patches were doing, getting rid of all get_attr_... calls 
inside conditions.  I'm especially worried about a sequence of get_attr() 
calls in conditions where you can remove only the first one, if at all.

This would mean to evaluate some get_attr calls more often than in an 
unmodified insn-attrtab, but in past measurements this didn't change 
performance (and reduced code size even more).

> Ok to commit (assuming the speedup patch is approved;

Mark approved your first speedup patch.


Ciao,
Michael.