[PATCH, rs6000] Refactor FP vector comparison operators

Kewen.Lin linkw@linux.ibm.com
Mon Nov 25 07:29:00 GMT 2019


Hi Segher,

on 2019/11/23 脡脧脦莽12:08, Segher Boessenkool wrote:
> Hi!
>> 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.
> 
> (You don't have to wrap so early...  Line length is 80 columns.)
> 
>> +;; code iterators and attributes for vector FP comparison operators:
>> +(define_code_iterator
>> +	vector_fp_comparison_simple [lt le ne ungt unge unlt unle])
>> +(define_code_iterator
>> +	vector_fp_comparison_complex [ltgt uneq unordered ordered])
> 
> Please indent by two spaces, not a tab.
> 
>> +; For lt/le/ne/ungt/unge/unlt/unle:
>> +; lt(a,b)   = gt(b,a)
>> +; le(a,b)   = ge(b,a)
>> +; unge(a,b) = ~lt(a,b)
>> +; unle(a,b) = ~gt(a,b)
>> +; ne(a,b)   = ~eq(a,b)
>> +; ungt(a,b) = ~le(a,b)
>> +; unlt(a,b) = ~ge(a,b)
>> +(define_insn_and_split "vector_<code><mode>"
>>    [(set (match_operand:VEC_F 0 "vfloat_operand")
>> +	(vector_fp_comparison_simple:VEC_F
>> +			   (match_operand:VEC_F 1 "vfloat_operand")
>> +			   (match_operand:VEC_F 2 "vfloat_operand")))]
> 
> Indent is weird here as well.
> 
>> +; For ltgt/uneq/ordered/unordered:
>> +; ltgt: gt(a,b) | gt(b,a)
>> +; uneq: ~(gt(a,b) | gt(b,a))
>> +; ordered: ge(a,b) | ge(b,a)
>> +; unordered: ~(ge(a,b) | ge(b,a))
>> +(define_insn_and_split "vector_<code><mode>"
>>    [(set (match_operand:VEC_F 0 "vfloat_operand")
>> +	(vector_fp_comparison_complex:VEC_F
>> +			   (match_operand:VEC_F 1 "vfloat_operand")
>> +			   (match_operand:VEC_F 2 "vfloat_operand")))]
>> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode) && can_create_pseudo_p ()"
>>    "#"
>> +  "&& can_create_pseudo_p ()"
> 
> So hrm, if we do that here we can as well do that in the previous
> splitter as well (and not do the operands[0] thing) (sorry for going
> back on this -- I have said that before haven't I?)
> 
>> +  switch (cond)
>> +    {
>> +    case LTGT:
>> +      cond = GT;
>> +      break;
>> +    case ORDERED:
>> +      cond = GE;
>> +      break;
>> +    default:
>> +      gcc_unreachable ();
>> +    }
> 
> It feels a bit lighter (and is shorter) if you write
> 
>   if (cond == LTGT)
>     cond = GT;
>   else if (cond == ORDERED)
>     cond = GE;
>   else
>     gcc_unreachable ();
> 
> 
> Okay for trunk with those trivialities taken care of one way or the
> other.  Thanks!
> 

Updated it as all your suggestions above and committed in r278665.

Thanks again!

BR,
Kewen



More information about the Gcc-patches mailing list