On Fri, May 12, 2017 at 8:34 PM, Jeff Law <l...@redhat.com> wrote:
> On 05/10/2017 01:05 PM, Uros Bizjak wrote:
>>
>> On Wed, May 10, 2017 at 5:18 PM, Uros Bizjak <ubiz...@gmail.com> wrote:
>>>
>>> On Wed, May 10, 2017 at 4:27 PM, Jakub Jelinek <ja...@redhat.com> wrote:
>>>>
>>>> On Tue, May 09, 2017 at 06:06:47PM +0200, Uros Bizjak wrote:
>>>>>
>>>>> Attached patch enables post-reload compare elimination pass by
>>>>> providing expected patterns (duplicates of existing patterns with
>>>>> setters of reg and flags switched in the parallel) for flag setting
>>>>> arithmetic instructions.
>>>>>
>>>>> The merge triggers more than 3000 times during the gcc bootstrap,
>>>>> mostly in cases where intervening memory load or store prevents
>>>>> combine from merging the arithmetic insn and the following compare.
>>>>>
>>>>> Also, some recent linux x86_64 defconfig build results in ~200 merges,
>>>>> removing ~200 test/cmp insns. Not much, but I think the results still
>>>>> warrant the pass to be enabled.
>>>>
>>>>
>>>> Isn't the right fix instead to change the compare-elim.c pass to either
>>>> accept both reg vs. flags orderings in parallel, or both depending
>>>> on some target hook, or change it to the order i386.md and most other
>>>> major targets use and just fix up mn10300/rx (and aarch64?) to use the
>>>> same
>>>> order?
>>
>>
>> Attached patch changes compare-elim.c order to what i386.md expects.
>>
>> Thoughts?
>
> I think with an appropriate change to the canonicalization rules in the
> manual this is fine.
>
> I've got the visium, rx and mn103 patches handy to ensure they don't
> regress. aarch64 doesn't seem to be affected either way but I didn't
> investigate why -- I expected it to improve with your change.
>
> I'll write up a ChangeLog and commit the mn103, rx and visium changes after
> you commit the compare-elim & documentation bits.
Thanks!
I went ahead and commit the patch without documentation change (which
I plan to submit in a separate patch ASAP), with the following
ChangeLog:
2017-05-12 Uros Bizjak <ubiz...@gmail.com>
* compare-elim.c (try_eliminate_compare): 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.
Re-bootstrapped and re-tested on x86_64-linux-gnu {,-m32}.
Committed to mainline SVN.
Uros.
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c (revision 247978)
+++ config/i386/i386.c (working copy)
@@ -51805,6 +51826,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: compare-elim.c
===================================================================
--- compare-elim.c (revision 247978)
+++ 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,24 @@ 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);
+ /* 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);
+
if (!apply_change_group ())
return false;
