Re: [PATCH 2/2] ifcvt: Allow more operations in multiple set if conversion

[Robin Dapp <rdapp.gcc@gmail.com>]

Hi Manolis,

that looks like a nice enhancement of what's already possible.  The concern
I had some years back already was that this function would eventually
grow and cannibalize on some of what the other functions in ifcvt already
do :)  At some point we really should unify but that's not within the
scope of this patch.

IMHO we're already pretty far towards general "conditional execution"
with conditional increments, selects and so on (and the function is still
called "_noce") and historically the cond_exec functions would have
taken care of that.  To my knowledge though, none of the major backends
implements anything like (cond_exec ...) anymore and relies on bit-twiddling
tricks to generate the conditional instructions.

Have you checked whether cond_exec and others could be adjusted to
handle the conditional instructions you want to see?  They don't perform
full cost comparison though but just count.

I would expect a bit of discussion around that but from a first look
I don't have major concerns.

> -/* Return true iff basic block TEST_BB is comprised of only
> -   (SET (REG) (REG)) insns suitable for conversion to a series
> -   of conditional moves.  Also check that we have more than one set
> -   (other routines can handle a single set better than we would), and
> -   fewer than PARAM_MAX_RTL_IF_CONVERSION_INSNS sets.  While going
> +/* Return true iff basic block TEST_BB is suitable for conversion to a
> +   series of conditional moves.  Also check that we have more than one

Might want to change the "conditional moves" while you're at it.

>  
> -      if (!((REG_P (src) || CONSTANT_P (src))
> -	    || (GET_CODE (src) == SUBREG && REG_P (SUBREG_REG (src))
> -	      && subreg_lowpart_p (src))))
> +      /* Allow a wide range of operations and let the costing function decide
> +	 if the conversion is worth it later.  */
> +      enum rtx_code code = GET_CODE (src);
> +      if (!(CONSTANT_P (src)
> +	    || code == REG
> +	    || code == SUBREG
> +	    || code == ZERO_EXTEND
> +	    || code == SIGN_EXTEND
> +	    || code == NOT
> +	    || code == NEG
> +	    || code == PLUS
> +	    || code == MINUS
> +	    || code == AND
> +	    || code == IOR
> +	    || code == MULT
> +	    || code == ASHIFT
> +	    || code == ASHIFTRT
> +	    || code == NE
> +	    || code == EQ
> +	    || code == GE
> +	    || code == GT
> +	    || code == LE
> +	    || code == LT
> +	    || code == GEU
> +	    || code == GTU
> +	    || code == LEU
> +	    || code == LTU
> +	    || code == COMPARE))

We're potentially checking many more patterns than before.  Maybe it
would make sense to ask the backend whether it has a pattern for
the respective code?

Regards
 Robin