Re: [PATCH][Aarch64] v2: Arithmetic overflow addv patterns [Patch 2/4]

[Richard Sandiford <richard dot sandiford at arm dot com>]

Michael Collison <Michael.Collison@arm.com> writes:
> +(define_expand "uaddv<mode>4"
> +  [(match_operand:GPI 0 "register_operand")
> +   (match_operand:GPI 1 "register_operand")
> +   (match_operand:GPI 2 "register_operand")
> +   (label_ref (match_operand 3 "" ""))]
> +  ""
> +{
> +  emit_insn (gen_add<mode>3_compareC (operands[0], operands[1], operands[2]));
> +  aarch64_gen_unlikely_cbranch (NE, CC_Cmode, operands[3]);
> +
> +  DONE;
> +})
> +
> +

Nit: stray extra line.

>  (define_expand "addti3"
>    [(set (match_operand:TI 0 "register_operand" "")
>  	(plus:TI (match_operand:TI 1 "register_operand" "")
> -		 (match_operand:TI 2 "register_operand" "")))]
> +		 (match_operand:TI 2 "aarch64_reg_or_imm" "")))]
>    ""
>  {
> -  rtx low = gen_reg_rtx (DImode);
> -  emit_insn (gen_adddi3_compareC (low, gen_lowpart (DImode, operands[1]),
> -				  gen_lowpart (DImode, operands[2])));
> +  rtx low_dest,op1_low,op2_low,high_dest,op1_high,op2_high;

Spaces after commas (sorry)

[...]

> @@ -1837,10 +1946,70 @@
>    [(set_attr "type" "alus_sreg")]
>  )
>  
> +;; Note that since we're sign-extending, match the immediate in GPI
> +;; rather than in DWI.  Since CONST_INT is modeless, this works fine.
> +(define_insn "*add<mode>3_compareV_cconly_imm"
> +  [(set (reg:CC_V CC_REGNUM)
> +	(compare:CC_V
> +	  (plus:<DWI>
> +	    (sign_extend:<DWI> (match_operand:GPI 0 "register_operand" "r,r"))
> +	    (match_operand:GPI 1 "aarch64_plus_immediate" "I,J"))
> +	  (sign_extend:<DWI> (plus:GPI (match_dup 0) (match_dup 1)))))]

Real reason for replying is: this is a neat trick, but I think it's
only an accident that genrecog doesn't reject the mode on operand 1.
PLUSes can't have mismatched modes since since rtl doesn't have the
sign information to do a conversion.

IMO we should use a similar structure to the zero_extends:

	  (plus:<DWI>
	    (zero_extend:<DWI> (match_operand:GPI 0 "register_operand" "r,r"))
	    (match_operand:<DWI> 1 "const_scalar_int_operand" ""))
	  (sign_extend:<DWI>
	    (plus:GPI
	      (match_dup 0)
	      (match_operand:GPI 2 "aarch64_plus_immediate" "I,J")))))

but with the simpler check:

  INTVAL (operands[1]) == INTVAL (operands[2])

Thanks,
Richard