Hi!
On Tue, Nov 12, 2019 at 06:41:07PM +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])
Let's indent that differently?
(define_code_iterator
vector_fp_comparison_operator [lt le ne ungt unge unlt unle])
is nice I think.
> +; There are 14 possible vector FP comparison operators, gt and eq of them
> have
> +; been expanded above, so just support 12 remaining operators here.
>
> +; 0. For ge:
(Don't number comments please).
> +(define_expand "vector_ge<mode>"
> + [(set (match_operand:VEC_F 0 "vlogical_operand")
> + (ge:VEC_F (match_operand:VEC_F 1 "vlogical_operand")
> + (match_operand:VEC_F 2 "vlogical_operand")))]
> "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode)"
> + "")
This already exists? Line 764 in vector.md?
> +; 1. 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_operator: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 ()"
> + [(pc)]
> {
> + enum rtx_code cond = <CODE>;
> + rtx res = gen_reg_rtx (<MODE>mode);
> + bool need_invert = false;
> + switch (cond)
> + {
> + case LT:
> + case LE:
> + cond = swap_condition (cond);
> + std::swap (operands[1], operands[2]);
> + break;
> + case UNLE:
> + case UNLT:
> + case NE:
> + case UNGE:
> + case UNGT:
> + cond = reverse_condition_maybe_unordered (cond);
> + need_invert = true;
> + break;
> + default:
> + gcc_unreachable ();
> + }
> +
> + rtx comp = gen_rtx_fmt_ee (cond, <MODE>mode, operands[1], operands[2]);
> + emit_insn (gen_rtx_SET (res, comp));
> +
> + if (need_invert)
> + emit_insn (gen_one_cmpl<mode>2 (res, res));
> +
> + emit_insn (gen_rtx_SET (operands[0], res));
> })
Does this work for (say) ungt? I would do two switch statements: the
first only to do the invert, and then the second to do the swap (with
the modified cond!) So:
{
enum rtx_code cond = <CODE>;
bool need_invert = false;
if (cond == UNLE || cond == UNLT || cond == NE
|| cond == UNGE || cond == UNGT)
{
cond = reverse_condition_maybe_unordered (cond);
need_invert = true;
}
if (cond == LT || cond == LE)
{
cond = swap_condition (cond);
std::swap (operands[1], operands[2]);
}
rtx comp = gen_rtx_fmt_ee (cond, <MODE>mode, operands[1], operands[2]);
if (need_invert)
{
rtx res = gen_reg_rtx (<MODE>mode);
emit_insn (gen_rtx_SET (res, comp));
emit_insn (gen_one_cmpl<mode>2 (operands[0], res));
}
else
emit_insn (gen_rtx_SET (operands[0], comp));
})
> +; 2. 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)
You could write that as
~(ge(a,b) | ge(b,a))
which may show the structure better?
> +(define_insn_and_split "vector_<code><mode>"
> [(set (match_operand:VEC_F 0 "vfloat_operand")
> + (vector_fp_extra_comparison_operator: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 ()"
This should be "&& can_create_pseudo ()".
Also, can_create_pseudo in the split condition can fail, in theory
anyway. It should be part of the insn condition, instead, and even
then it can fail: after the last split pass, but before reload. such
an insn can in principle be created, and then it sill be never split.
In this case, maybe you should add a scratch register; in the previous
case, you can reuse operands[0] for res instead of making a new reg
for it, if !can_create_pseudo ().
> {
> + enum rtx_code cond = <CODE>;
> + rtx res1 = gen_reg_rtx (<MODE>mode);
> + rtx res2 = gen_reg_rtx (<MODE>mode);
> + bool need_invert = false;
> + switch (cond)
> + {
> + case UNEQ:
> + need_invert = true;
> + /* Fall through. */
> + case LTGT:
> + cond = GT;
> + break;
> + case UNORDERED:
> + need_invert = true;
> + /* Fall through. */
> + case ORDERED:
> + cond = GE;
> + break;
> + default:
> + gcc_unreachable ();
> + }
> +
> + rtx comp1 = gen_rtx_fmt_ee (cond, <MODE>mode, operands[1], operands[2]);
> + emit_insn (gen_rtx_SET (res1, comp1));
> + rtx comp2 = gen_rtx_fmt_ee (cond, <MODE>mode, operands[2], operands[1]);
> + emit_insn (gen_rtx_SET (res2, comp2));
> +
> + emit_insn (gen_ior<mode>3 (res1, res1, res2));
> +
> + if (need_invert)
> + emit_insn (gen_one_cmpl<mode>2 (res1, res1));
> +
> + emit_insn (gen_rtx_SET (operands[0], res1));
> })
You can do this similarly:
{
enum rtx_code cond = <CODE>;
bool need_invert = false;
if (cond == UNORDERED || cond == UNEQ)
{
cond = reverse_condition_maybe_unordered (cond);
need_invert = true;
}
switch (cond)
{
case LTGT:
cond = GT;
break;
case ORDERED:
cond = GE;
break;
default:
gcc_unreachable ();
}
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);
emit_insn (gen_rtx_SET (operands[0], comp4));
}
else
emit_insn (gen_ior<mode>3 (operands[0], res1, res2));
})
(the result of a split does not get combine etc. optimisations anymore,
so you have to generate pretty much ideal code directly).
HTH, sorry for the late reply,
Segher
