On Fri, Aug 14, 2020 at 10:24 AM Hongtao Liu <crazy...@gmail.com> wrote:
>
>   Enable direct move between masks and gprs in pass_reload with
> consideration of cost model.
>
> Changelog
> gcc/
>         * config/i386/i386.c (inline_secondary_memory_needed):
>         No memory is needed between mask regs and gpr.
>         (ix86_hard_regno_mode_ok): Add condition TARGET_AVX512F for
>         mask regno.
>         * config/i386/i386.h (enum reg_class): Add INT_MASK_REGS.
>         (REG_CLASS_NAMES): Ditto.
>         (REG_CLASS_CONTENTS): Ditto.
>         * config/i386/i386.md: Exclude mask register in
>         define_peephole2 which is available only for gpr.
>
> gcc/testsuites/
>         * gcc.target/i386/pr71453-1.c: New tests.
>         * gcc.target/i386/pr71453-2.c: Ditto.
>         * gcc.target/i386/pr71453-3.c: Ditto.
>         * gcc.target/i386/pr71453-4.c: Ditto.

@@ -18571,9 +18571,7 @@ inline_secondary_memory_needed (machine_mode
mode, reg_class_t class1,
       || MAYBE_SSE_CLASS_P (class1) != SSE_CLASS_P (class1)
       || MAYBE_SSE_CLASS_P (class2) != SSE_CLASS_P (class2)
       || MAYBE_MMX_CLASS_P (class1) != MMX_CLASS_P (class1)
-      || MAYBE_MMX_CLASS_P (class2) != MMX_CLASS_P (class2)
-      || MAYBE_MASK_CLASS_P (class1) != MASK_CLASS_P (class1)
-      || MAYBE_MASK_CLASS_P (class2) != MASK_CLASS_P (class2))
+      || MAYBE_MMX_CLASS_P (class2) != MMX_CLASS_P (class2))
     {
       gcc_assert (!strict || lra_in_progress);
       return true;

No, this is still needed, the reason is explained in the comment above
inline_secondary_memory_needed:

   The function can't work reliably when one of the CLASSES is a class
   containing registers from multiple sets.  We avoid this by never combining
   different sets in a single alternative in the machine description.
   Ensure that this constraint holds to avoid unexpected surprises.

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index b24a4557871..74d207c3711 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -15051,7 +15051,7 @@
    (parallel [(set (reg:CC FLAGS_REG)
            (unspec:CC [(match_dup 0)] UNSPEC_PARITY))
           (clobber (match_dup 0))])]
-  ""
+  "!MASK_REGNO_P (REGNO (operands[0]))"
   [(set (reg:CC FLAGS_REG)
     (unspec:CC [(match_dup 1)] UNSPEC_PARITY))])

@@ -15072,6 +15072,7 @@
                (label_ref (match_operand 5))
                (pc)))]
   "REGNO (operands[2]) == REGNO (operands[3])
+   && !MASK_REGNO_P (REGNO (operands[1]))
    && peep2_reg_dead_p (3, operands[0])
    && peep2_reg_dead_p (3, operands[2])
    && peep2_regno_dead_p (4, FLAGS_REG)"

Actually, there are several (historic?) peephole2 patterns that assume
register_operand means only integer registers. Just change
register_operand to general_reg_operand and eventually
nonimmediate_operand to nonimmediate_gr_operand. Do not put additional
predicates into insn predicate.

Uros.

Reply via email to