This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH PR68542]


On Thu, Jan 28, 2016 at 2:37 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote:
> Thanks Richard.
>
> Uros,
>
> Could you please review back-end part of this patch?

No problem, but please in future CC me on the x86 patches, so I won't forgot.

+(define_expand "cbranch<mode>4"
+  [(set (reg:CC FLAGS_REG)
+    (compare:CC (match_operand:V48_AVX2 1 "nonimmediate_operand")
+            (match_operand:V48_AVX2 2 "register_operand")))
+

Please swap predicates, operand 1 should be register_operand and
operand 2 nonimmediate operand.

+   (set (pc) (if_then_else
+           (match_operator 0 "bt_comparison_operator"
+        [(reg:CC FLAGS_REG) (const_int 0)])
+           (label_ref (match_operand 3))
+           (pc)))]
+  "TARGET_AVX2"

PTEST was introduced with SSE4_1, I see no reason to limit this
transformation to AVX2. However, since you check MODE_VECTOR_INT in
the expander, it looks that the pattern should only handle integer
modes.

+  /* Handle special case - vector comparsion with boolean result, transform
+     it using ptest instruction.  */
+  if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT)
+    {
+      rtx lhs;

No need for the above variable, we will use tmp here.

+      rtx flag;
+      machine_mode p_mode = GET_MODE_SIZE (mode) == 32 ? V4DImode : V2DImode;

space here.

+      gcc_assert (code == EQ || code == NE);

+      if (!REG_P (op0))
+    op0 = force_reg (mode, op0);
+      if (!REG_P (op1))
+    op1 = force_reg (mode, op1);

No need for the above fixups, predicates already did the job.

+      /* Generate subtraction since we can't check that one operand is
+     zero vector.  */
+      lhs = gen_reg_rtx (mode);

tmp = ...

+      emit_insn (gen_rtx_SET (lhs, gen_rtx_MINUS (mode, op0, op1)));

Since we are only interested in equality, should we rather use XOR,
since it is commutative operator.

+      lhs = gen_rtx_SUBREG (p_mode, lhs, 0);

tmp = gen_lowpart (p_mode, tmp);

+      tmp = gen_rtx_SET (gen_rtx_REG (CCmode, FLAGS_REG),
+             gen_rtx_UNSPEC (CCmode,
+                     gen_rtvec (2, lhs, lhs),
+                     UNSPEC_PTEST));
+      emit_insn (tmp);

There is no need for a temporary here, just use

emit_insn (gen_rtx_SET ( ... ) ...

+      tmp = gen_rtx_fmt_ee (code, VOIDmode, flag, const0_rtx);
+      if (code == EQ)
+    tmp = gen_rtx_IF_THEN_ELSE (VOIDmode, tmp,
+                    gen_rtx_LABEL_REF (VOIDmode, label), pc_rtx);
+      else
+    tmp = gen_rtx_IF_THEN_ELSE (VOIDmode, tmp,
+                    pc_rtx, gen_rtx_LABEL_REF (VOIDmode, label));
+      emit_jump_insn (gen_rtx_SET (pc_rtx, tmp));
+      return;

Oh.... please use something involving std::swap here.

Uros.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]