Hi!
On Thu, Nov 21, 2019 at 09:15:19PM +0800, Kewen.Lin wrote:
> >> +;; 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?
Sure.
> 4) Append "DONE" at the end of define_insn_and_split.
Ooh did I miss that? That must have created some interesting code, heh.
> 2019-11-21 Kewen Lin <li...@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!
Segher
