diff mbox

[RFC,i386] : Enable post-reload compare elimination pass

Message ID CAFULd4bVXkoR2W0cvtc7mu=qhtpjEfaz1z9uFA6CP0xtHACsAw@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak May 11, 2017, 6:46 p.m. UTC
On Thu, May 11, 2017 at 8:28 PM, Jeff Law <law@redhat.com> wrote:

>> Attached patch changes compare-elim.c order to what i386.md expects.
>>
>> Thoughts?
>
> Haven't looked at the patch itself.  But I do have the necessary bits to
> convert the mn103 port.  It was slightly more than just fixing the md file,
> but nothing significant or time consuming.  The net result is 100% identical
> code for newlib before your patch vs after your patch w/mn103 converted.
>
> Hell, it was easy enough, I'll take a cut at the rx port.

Attached to this message, please find a version of the patch that
should ease transition of other targets by declaring  and handling a
temporary macro. This way, aarch64 can also test what happens when
postreload cmp elimination goes live for real.

2017-05-11  Uros Bizjak  <ubizjak@gmail.com>

    * compare-elim.c (try_eliminate_compare)
    [TARGET_HAS_ALTERNATIVE_POSTRELOAD_EMBEDDED_COMPARE_PATTERNS]:
    Canonicalize operation with embedded compare to
    [(set (reg:CCM) (compare:CCM (operation) (immediate)))
     (set (reg) (operation)].

    * config/i386/i386.c (TARGET_FLAGS_REGNUM): New define.
    * config/i386/i386.h
    (TARGET_HAS_ALTERNATIVE_POSTRELOAD_EMBEDDED_COMPARE_PATTERNS): Ditto.

Uros.

Comments

Jeff Law May 12, 2017, 6:13 a.m. UTC | #1
On 05/11/2017 12:46 PM, Uros Bizjak wrote:
> On Thu, May 11, 2017 at 8:28 PM, Jeff Law <law@redhat.com> wrote:
> 
>>> Attached patch changes compare-elim.c order to what i386.md expects.
>>>
>>> Thoughts?
>>
>> Haven't looked at the patch itself.  But I do have the necessary bits to
>> convert the mn103 port.  It was slightly more than just fixing the md file,
>> but nothing significant or time consuming.  The net result is 100% identical
>> code for newlib before your patch vs after your patch w/mn103 converted.
>>
>> Hell, it was easy enough, I'll take a cut at the rx port.
> 
> Attached to this message, please find a version of the patch that
> should ease transition of other targets by declaring  and handling a
> temporary macro. This way, aarch64 can also test what happens when
> postreload cmp elimination goes live for real.
> 
> 2017-05-11  Uros Bizjak  <ubizjak@gmail.com>
> 
>      * compare-elim.c (try_eliminate_compare)
>      [TARGET_HAS_ALTERNATIVE_POSTRELOAD_EMBEDDED_COMPARE_PATTERNS]:
>      Canonicalize operation with embedded compare to
>      [(set (reg:CCM) (compare:CCM (operation) (immediate)))
>       (set (reg) (operation)].
Bah.  No need for handling both -- it's easy enough to just test the 
target and see what's caught or missing.

So far the mn103, rx and visium ports, generate the same code 
before/after conversion with fairly simple patches.

So I really think we should test aarch64, then do final review.

jeff
diff mbox

Patch

Index: compare-elim.c
===================================================================
--- compare-elim.c	(revision 247914)
+++ compare-elim.c	(working copy)
@@ -45,9 +45,9 @@  along with GCC; see the file COPYING3.  If not see
    (3) If an insn of form (2) can usefully set the flags, there is
        another pattern of the form
 
-	[(set (reg) (operation)
-	 (set (reg:CCM) (compare:CCM (operation) (immediate)))]
-
+	[(set (reg:CCM) (compare:CCM (operation) (immediate)))
+	 (set (reg) (operation)]
+	 
        The mode CCM will be chosen as if by SELECT_CC_MODE.
 
    Note that unlike NOTICE_UPDATE_CC, we do not handle memory operands.
@@ -582,7 +582,7 @@  equivalent_reg_at_start (rtx reg, rtx_insn *end, r
 static bool
 try_eliminate_compare (struct comparison *cmp)
 {
-  rtx x, flags, in_a, in_b, cmp_src;
+  rtx flags, in_a, in_b, cmp_src;
 
   /* We must have found an interesting "clobber" preceding the compare.  */
   if (cmp->prev_clobber == NULL)
@@ -628,7 +628,8 @@  try_eliminate_compare (struct comparison *cmp)
      Validate that PREV_CLOBBER itself does in fact refer to IN_A.  Do
      recall that we've already validated the shape of PREV_CLOBBER.  */
   rtx_insn *insn = cmp->prev_clobber;
-  x = XVECEXP (PATTERN (insn), 0, 0);
+
+  rtx x = XVECEXP (PATTERN (insn), 0, 0);
   if (rtx_equal_p (SET_DEST (x), in_a))
     cmp_src = SET_SRC (x);
 
@@ -666,13 +667,28 @@  try_eliminate_compare (struct comparison *cmp)
     flags = gen_rtx_REG (cmp->orig_mode, targetm.flags_regnum);
 
   /* Generate a new comparison for installation in the setter.  */
-  x = copy_rtx (cmp_src);
-  x = gen_rtx_COMPARE (GET_MODE (flags), x, in_b);
-  x = gen_rtx_SET (flags, x);
+  rtx y = copy_rtx (cmp_src);
+  y = gen_rtx_COMPARE (GET_MODE (flags), y, in_b);
+  y = gen_rtx_SET (flags, y);
 
+#if TARGET_HAS_ALTERNATIVE_POSTRELOAD_EMBEDDED_COMPARE_PATTERNS
+  /* Canonicalize instruction to:
+     [(set (reg:CCM) (compare:CCM (operation) (immediate)))
+      (set (reg) (operation)]  */
+
+  rtvec v = rtvec_alloc (2);
+  RTVEC_ELT (v, 0) = y;
+  RTVEC_ELT (v, 1) = x;
+  
+  rtx pat = gen_rtx_PARALLEL (VOIDmode, v);
+  
   /* Succeed if the new instruction is valid.  Note that we may have started
      a change group within maybe_select_cc_mode, therefore we must continue. */
-  validate_change (insn, &XVECEXP (PATTERN (insn), 0, 1), x, true);
+  validate_change (insn, &PATTERN (insn), pat, true);
+#else
+  validate_change (insn, &XVECEXP (PATTERN (insn), 0, 1), y, true);
+#endif
+  
   if (!apply_change_group ())
     return false;
 
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 247914)
+++ config/i386/i386.c	(working copy)
@@ -52043,6 +52043,8 @@  ix86_run_selftests (void)
 #undef TARGET_ADDRESS_COST
 #define TARGET_ADDRESS_COST ix86_address_cost
 
+#undef TARGET_FLAGS_REGNUM
+#define TARGET_FLAGS_REGNUM FLAGS_REG
 #undef TARGET_FIXED_CONDITION_CODE_REGS
 #define TARGET_FIXED_CONDITION_CODE_REGS ix86_fixed_condition_code_regs
 #undef TARGET_CC_MODES_COMPATIBLE
Index: config/i386/i386.h
===================================================================
--- config/i386/i386.h	(revision 247914)
+++ config/i386/i386.h	(working copy)
@@ -2658,6 +2658,9 @@  extern void debug_dispatch_window (int);
 
 #define TARGET_SUPPORTS_WIDE_INT 1
 
+/* Temporary macro to ease transition, should be removed ASAP.  */
+#define TARGET_HAS_ALTERNATIVE_POSTRELOAD_EMBEDDED_COMPARE_PATTERNS 1
+
 /*
 Local variables:
 version-control: t