[PATCH, rs6000] Refactor FP vector comparison operators

Segher Boessenkool segher@kernel.crashing.org
Mon Nov 11 13:05:00 GMT 2019


Hi!

On Mon, Nov 11, 2019 at 03:40:51PM +0800, Kewen.Lin wrote:
> This is a subsequent patch to refactor the existing float point
> vector comparison operator supports.  The patch to fix PR92132
> supplemented vector float point comparison by exposing the names
> for unordered/ordered/uneq/ltgt and adding ungt/unge/unlt/unle/
> ne.  As Segher pointed out, some patterns can be refactored
> together.  The main link on this is: 
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00452.html
> 
> 
> The refactoring mainly follows the below patterns:
> 
> pattern 1:
>   lt(a,b) = gt(b,a)
>   le(a,b) = ge(b,a)

This is done by swap_condition normally.

> pattern 2:
>   unge(a,b) = ~gt(b,a)
>   unle(a,b) = ~gt(a,b)
>   ne(a,b)   = ~eq(a,b)
>   ungt(a,b) = ~ge(b,a)
>   unlt(a,b) = ~ge(a,b)

This is reverse_condition_maybe_unordered (and a swap, in two cases).

> pattern 3:
>   ltgt: gt(a,b) | gt(b,a)
>   ordered: ge(a,b) | ge(b,a)

That's the only interesting one :-)

> pattern 4:
>   uneq: ~gt(a,b) & ~gt(b,a)
>   unordered: ~ge(a,b) & ~ge(b,a)

That is 3, reversed.

> Naming the code iterators and attributes are really knotty for me :(.

Maybe the above helps :-)

> 	(vector_<code><mode> for VEC_F and vec_fp_cmp1): New
> 	define_and_split.

> +;; code iterators and attributes for vector FP comparison operators:
> +
> +;; 1. lt and le.
> +(define_code_iterator vec_fp_cmp1 [lt le])
> +(define_code_attr vec_fp_cmp1_attr [(lt "gt")
> +				    (le "ge")])

So this is just (a subset of) swap_condition.

(I'm reordering stuff)

> +; 1. For lt and le:
> +; lt(a,b) = gt(b,a)
> +; le(a,b) = ge(b,a)
> +(define_insn_and_split "vector_<code><mode>"
>    [(set (match_operand:VEC_F 0 "vfloat_operand")
> +	(vec_fp_cmp1:VEC_F (match_operand:VEC_F 1 "vfloat_operand")
> +			   (match_operand:VEC_F 2 "vfloat_operand")))]
>    "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)"
>    "#"
>    ""
> +  [(set (match_dup 0)
> +	(<vec_fp_cmp1_attr>:VEC_F (match_dup 2)
> +				  (match_dup 1)))]
>  {
>  })

Empty preparation statements (the {}) can just be left out completely.

The split condition "" is incorrect, it should be "&& 1": if it starts
with "&&", the insn condition is included.

> +; 2. unge, unle, ne, ungt and unlt.
> +(define_code_iterator vec_fp_cmp2 [unge unle ne ungt unlt])
> +(define_code_attr vec_fp_cmp2_attr [(unge "gt")
> +				    (unle "gt")
> +				    (ne   "eq")
> +				    (ungt "ge")
> +				    (unlt "ge")])

> +; 2. For unge, unle, ne, ungt and unlt:
> +; unge(a,b) = ~gt(b,a)
> +; unle(a,b) = ~gt(a,b)
> +; ne(a,b)   = ~eq(a,b)
> +; ungt(a,b) = ~ge(b,a)
> +; unlt(a,b) = ~ge(a,b)
> +(define_insn_and_split "vector_<code><mode>"
>    [(set (match_operand:VEC_F 0 "vfloat_operand")
> +	(vec_fp_cmp2:VEC_F (match_operand:VEC_F 1 "vfloat_operand")
> +			   (match_operand:VEC_F 2 "vfloat_operand")))]
>    "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)"
>    "#"
>    ""
>    [(set (match_dup 3)
> +	(<vec_fp_cmp2_attr>:VEC_F (match_dup 1)
> +				  (match_dup 2)))
>     (set (match_dup 0)
> +	(not:VEC_F (match_dup 3)))]
>  {
>    operands[3] = gen_reg_rtx (<MODE>mode);
> +
> +  if (<CODE> == UNGE || <CODE> == UNGT)
> +    std::swap (operands[1], operands[2]);
>  })

So maybe it is simplest to *do* use match_operator here, handle all of
lt gt le ge eq unge unle ungt unlt ne  in one define_expand, which then
swaps the condition and the args, and expand the extra not for the five
where that is needed?


> +;; 3. ltgt and ordered.
> +(define_code_iterator vec_fp_cmp3 [ltgt ordered])
> +(define_code_attr vec_fp_cmp3_attr [(ltgt "gt")
> +				    (ordered "ge")])
> +
> +;; 4. uneq and unordered.
> +(define_code_iterator vec_fp_cmp4 [uneq unordered])
> +(define_code_attr vec_fp_cmp4_attr [(uneq "gt")
> +				    (unordered "ge")])

And then another one for  ltgt uneq ordered unordered  perhaps?

So you'll need to define two new predicates then.  Something like
vector_fp_comparison_operator and, erm, vector_fp_extra_comparison_operator,
which kind of sucks as a name, but there is only one ;-)

Sorry for sending you first one way, and then back the other.


Segher



More information about the Gcc-patches mailing list