On Fri, Jan 14, 2022 at 11:56 PM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> C++20:
> #include <compare>
> auto cmp4way(double a, double b)
> {
> return a <=> b;
> }
> expands to:
> ucomisd %xmm1, %xmm0
> jp .L8
> movl $0, %eax
> jne .L8
> .L2:
> ret
> .p2align 4,,10
> .p2align 3
> .L8:
> comisd %xmm0, %xmm1
> movl $-1, %eax
> ja .L2
> ucomisd %xmm1, %xmm0
> setbe %al
> addl $1, %eax
> ret
> That is 3 comparisons of the same operands.
> The following patch improves it to just one comparison:
> comisd %xmm1, %xmm0
> jp .L4
> seta %al
> movl $0, %edx
> leal -1(%rax,%rax), %eax
> cmove %edx, %eax
> ret
> .L4:
> movl $2, %eax
> ret
> While a <=> b expands to a == b ? 0 : a < b ? -1 : a > b ? 1 : 2
> where the first comparison is equality and this shouldn't raise
> exceptions on qNaN operands, if the operands aren't equal (which
> includes unordered cases), then it immediately performs < or >
> comparison and that raises exceptions even on qNaNs, so we can just
> perform a single comparison that raises exceptions on qNaN.
> As the 4 different cases are encoded as
> ZF CF PF
> 1 1 1 a unordered b
> 0 0 0 a > b
> 0 1 0 a < b
> 1 0 0 a == b
> we can emit optimal sequence of comparions, first jp
> for the unordered case, then je for the == case and finally jb
> for the < case.
>
> The patch pattern recognizes spaceship-like comparisons during
> widening_mul if the spaceship optab is implemented, and replaces
> thoose comparisons with comparisons of .SPACESHIP ifn which returns
> -1/0/1/2 based on the comparison. This seems to work well both for the
> case of just returning the -1/0/1/2 (when we have just a common
> successor with a PHI) or when the different cases are handled with
> various other basic blocks. The testcases cover both of those cases,
> the latter with different function calls in those.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2022-01-14 Jakub Jelinek <ja...@redhat.com>
>
> PR target/103973
> * tree-cfg.h (cond_only_block_p): Declare.
> * tree-ssa-phiopt.c (cond_only_block_p): Move function to ...
> * tree-cfg.c (cond_only_block_p): ... here. No longer static.
> * optabs.def (spaceship_optab): New optab.
> * internal-fn.def (SPACESHIP): New internal function.
> * internal-fn.h (expand_SPACESHIP): Declare.
> * internal-fn.c (expand_PHI): Formatting fix.
> (expand_SPACESHIP): New function.
> * tree-ssa-math-opts.c (optimize_spaceship): New function.
> (math_opts_dom_walker::after_dom_children): Use it.
> * config/i386/i386.md (spaceship<mode>3): New define_expand.
> * config/i386/i386-protos.h (ix86_expand_fp_spaceship): Declare.
> * config/i386/i386-expand.c (ix86_expand_fp_spaceship): New function.
> * doc/md.texi (spaceship@var{m}3): Document.
>
> * gcc.target/i386/pr103973-1.c: New test.
> * gcc.target/i386/pr103973-2.c: New test.
> * gcc.target/i386/pr103973-3.c: New test.
> * gcc.target/i386/pr103973-4.c: New test.
> * g++.target/i386/pr103973-1.C: New test.
> * g++.target/i386/pr103973-2.C: New test.
> * g++.target/i386/pr103973-3.C: New test.
> * g++.target/i386/pr103973-4.C: New test.
>--- gcc/config/i386/i386.md.jj 2022-01-14 11:51:34.432384170 +0100
> +++ gcc/config/i386/i386.md 2022-01-14 18:22:41.140906449 +0100
> @@ -23886,6 +23886,18 @@ (define_insn "hreset"
> [(set_attr "type" "other")
> (set_attr "length" "4")])
>
> +;; Spaceship optimization
> +(define_expand "spaceship<mode>3"
> + [(match_operand:SI 0 "register_operand")
> + (match_operand:MODEF 1 "cmp_fp_expander_operand")
> + (match_operand:MODEF 2 "cmp_fp_expander_operand")]
> + "(TARGET_80387 || (SSE_FLOAT_MODE_P (<MODE>mode) && TARGET_SSE_MATH))
> + && TARGET_CMOVE && TARGET_IEEE_FP"
Is there a reason that this pattern is limited to TARGET_IEEE_FP?
During the expansion in ix86_expand_fp_spaceship, we can just skip
jump on unordered, while ix86_expand_fp_compare will emit the correct
comparison mode depending on TARGET_IEEE_FP.
> +{
> + ix86_expand_fp_spaceship (operands[0], operands[1], operands[2]);
> + DONE;
> +})
> +
> (include "mmx.md")
> (include "sse.md")
> (include "sync.md")
> --- gcc/config/i386/i386-expand.c.jj 2022-01-14 11:51:34.429384213 +0100
> +++ gcc/config/i386/i386-expand.c 2022-01-14 21:02:09.446437810 +0100
> @@ -2879,6 +2879,46 @@ ix86_expand_setcc (rtx dest, enum rtx_co
> emit_insn (gen_rtx_SET (dest, ret));
> }
>
> +/* Expand floating point op0 <=> op1 if NaNs are honored. */
> +
> +void
> +ix86_expand_fp_spaceship (rtx dest, rtx op0, rtx op1)
> +{
> + gcc_checking_assert (ix86_fp_comparison_strategy (GT) == IX86_FPCMP_COMI);
> + rtx gt = ix86_expand_fp_compare (GT, op0, op1);
> + rtx l0 = gen_label_rtx ();
> + rtx l1 = gen_label_rtx ();
> + rtx l2 = gen_label_rtx ();
> + rtx lend = gen_label_rtx ();
> + rtx un = gen_rtx_fmt_ee (UNORDERED, VOIDmode,
> + gen_rtx_REG (CCFPmode, FLAGS_REG), const0_rtx);
> + rtx tmp = gen_rtx_IF_THEN_ELSE (VOIDmode, un,
> + gen_rtx_LABEL_REF (VOIDmode, l2), pc_rtx);
> + rtx_insn *jmp = emit_jump_insn (gen_rtx_SET (pc_rtx, tmp));
> + add_reg_br_prob_note (jmp, profile_probability:: very_unlikely ());
Please also add JUMP_LABEL to the insn.
> + rtx eq = gen_rtx_fmt_ee (UNEQ, VOIDmode,
> + gen_rtx_REG (CCFPmode, FLAGS_REG), const0_rtx);
> + tmp = gen_rtx_IF_THEN_ELSE (VOIDmode, eq,
> + gen_rtx_LABEL_REF (VOIDmode, l0), pc_rtx);
> + jmp = emit_jump_insn (gen_rtx_SET (pc_rtx, tmp));
> + add_reg_br_prob_note (jmp, profile_probability::unlikely ());
> + tmp = gen_rtx_IF_THEN_ELSE (VOIDmode, gt,
> + gen_rtx_LABEL_REF (VOIDmode, l1), pc_rtx);
> + jmp = emit_jump_insn (gen_rtx_SET (pc_rtx, tmp));
> + add_reg_br_prob_note (jmp, profile_probability::even ());
> + emit_move_insn (dest, constm1_rtx);
> + emit_jump (lend);
> + emit_label (l0);
and LABEL_NUSES label.
> + emit_move_insn (dest, const0_rtx);
> + emit_jump (lend);
> + emit_label (l1);
> + emit_move_insn (dest, const1_rtx);
> + emit_jump (lend);
> + emit_label (l2);
> + emit_move_insn (dest, const2_rtx);
> + emit_label (lend);
> +}
> +
> /* Expand comparison setting or clearing carry flag. Return true when
> successful and set pop for the operation. */
> static bool
Uros.
