Re: [PATCH, rs6000] Refactor FP vector comparison operators

["Kewen.Lin" <linkw at linux dot ibm dot com>]

Hi Segher,

on 2019/11/20 下午10:06, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, Nov 20, 2019 at 03:31:36PM +0800, Kewen.Lin wrote:
> Yeah.  Just doing can_create_pseudo in the insn condition (and in the
> split condition, via &&) will work -- there just is this window of
> failure you should be aware of, and we need to do something about that,
> eventually.  It's a very general problem.  There are many places in
> rs6000 where we already do evil things with this (and some much worse
> than this even), so one more won't hurt too much.
> 
> It does need can_create_pseudo in the split condition then though.
> 

Done.

>>>   rtx comp1 = gen_rtx_fmt_ee (cond, <MODE>mode, operands[1], operands[2]);
>>>   rtx res1 = gen_reg_rtx (<MODE>mode);
>>>   emit_insn (gen_rtx_SET (res1, comp1));
>>>   rtx comp2 = gen_rtx_fmt_ee (cond, <MODE>mode, operands[2], operands[1]);
>>>   rtx res2 = gen_reg_rtx (<MODE>mode);
>>>   emit_insn (gen_rtx_SET (res2, comp2));
>>>
>>>   if (need_invert)
>>>     {
>>>       rtx comp3 = gen_rtx_fmt_ee (IOR, <MODE>mode, res1, res2);
>>>       rtx comp4 = gen_rtx_fmt_e (NOT, <MODE>mode, comp3);
>>
>> I had to add one more gen_rtx_SET for the IOR here, otherwise it gets the error
>> on the pattern can't be recognized.
> 
> Oh, sorry.  The canonical way of writing a NOR is as an ANDCC, so this
> should be
> 
>       rtx not1 = gen_rtx_fmt_e (NOT, <MODE>mode, res1);
>       rtx not2 = gen_rtx_fmt_e (NOT, <MODE>mode, res2);
>       rtx comp3 = gen_rtx_fmt_ee (AND, <MODE>mode, not1, not2);
>       emit_insn (gen_rtx_SET (operands[0], comp3));
> 

Thanks for the example, updated.

>> diff --git a/gcc/config/rs6000/vector.md b/gcc/config/rs6000/vector.md
>> index b132037..deeab9f 100644
>> --- a/gcc/config/rs6000/vector.md
>> +++ b/gcc/config/rs6000/vector.md
>> @@ -107,6 +107,12 @@
>>  				 (smin "smin")
>>  				 (smax "smax")])
>>  
>> +;; code iterators and attributes for vector FP comparison operators:
>> +(define_code_iterator
>> +	vector_fp_comparison_operator [lt le ne ungt unge unlt unle])
> 
> Can you change this name so it is clear it is just the simple ones?
> 

Renamed as vector_fp_comparison_simple and vector_fp_comparison_complex,
does it look better?

New version attached, compared with last version:
  1) Change code iterator names to vector_fp_comparison_simple
     and vector_fp_comparison_complex.
  2) Add can_create_pseudo in the split condition.
  3) Update NOR generation with AND (NOT, NOT).
  4) Append "DONE" at the end of define_insn_and_split.

Bootstrapped and regress tested on powerpc64le-linux-gnu.

BR,
Kewen

----
gcc/ChangeLog

2019-11-21 Kewen Lin  <linkw@gcc.gnu.org>

	* config/rs6000/vector.md (vector_fp_comparison_simple):
	New code iterator.
	(vector_fp_comparison_complex): Likewise.
	(vector_<code><mode> for VEC_F and
	vector_fp_comparison_simple): New define_and_split.
	(vector_<code><mode> for VEC_F and
	vector_fp_comparison_complex): Likewise.
	(vector_lt<mode> for VEC_F): Refactor with
	vector_fp_comparison_simple.
	(vector_le<mode> for VEC_F): Likewise.
	(vector_unge<mode> for VEC_F): Likewise.
	(vector_unle<mode> for VEC_F): Likewise.
	(vector_ne<mode> for VEC_F): Likewise.
	(vector_ungt<mode> for VEC_F): Likewise.
	(vector_unlt<mode> for VEC_F): Likewise.
	(vector_ltgt<mode> for VEC_F): Refactor with
	vector_fp_comparison_complex.
	(vector_ordered<mode> for VEC_F): Likewise.
	(vector_uneq<mode> for VEC_F): Likewise.
	(vector_unordered<mode> for VEC_F): Likewise.

Attachment: fp_vec_cmp_refactor_v3.diff
Description: Text document