Re: [Patch] Teach RTL ifcvt to handle multiple simple set instructions

[Bernd Schmidt <bernds_cb1 at t-online dot de>]

On 09/08/2015 04:53 PM, James Greenhalgh wrote:
> One big question I have with this patch is how I ought to write a meaningful
> cost model I've used. It seems like yet another misuse of RTX costs, and
> another bit of stuff for targets to carefully balance. Now, if the
> relative cost of branches and conditional move instructions is not
> carefully managed, you may enable or disable these optimisations. This is
> probably acceptable, but I dislike adding more and more gotcha's to
> target costs, as I get bitten by them hard enough as is!

The code you have seems reasonable, except that for compile time it 
might make sense to not even attempt the optimization if the number of 
sets is too large. I'm not too worried about that, but maybe you could 
bail out early if your cost estimate goes too much above the branch cost.

> +      /* If we were supposed to read from an earlier write in this block,
> +	 we've changed the register allocation.  Rewire the read.  While
> +	 we are looking, also try to catch a swap idiom.  */

So this is one interesting case; do you also have to worry about others 
(such as maybe setting the same register multiple times)?

> +  /* We must have seen some sort of insn to insert, otherwise we were
> +     given an empty BB to convert, and we can't handle that.  */
> +  if (unmodified_insns.is_empty ())
> +    {
> +      end_sequence ();
> +      return FALSE;
> +    }

Looks like some of the error conditions are tested twice across the two 
new functions? I think it would be better to get rid of one copy or turn 
the second one into a gcc_assert.

> No testcase provided, as currently I don't know of targets with a high
> enough branch cost to actually trigger the optimisation.

Hmm, so the code would not actually be used right now? In that case I'll 
leave it to others to decide whether we want to apply it. Other than the 
points above it looks OK to me.


Bernd