This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC PATCH, i386]: Enable post-reload compare elimination pass
On 2017.05.12 at 21:09 +0200, Uros Bizjak wrote:
> On Fri, May 12, 2017 at 8:34 PM, Jeff Law <law@redhat.com> wrote:
> > On 05/10/2017 01:05 PM, Uros Bizjak wrote:
> >>
> >> On Wed, May 10, 2017 at 5:18 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> >>>
> >>> On Wed, May 10, 2017 at 4:27 PM, Jakub Jelinek <jakub@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 <ubizjak@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.
This causes gcc.dg/atomic/c11-atomic-exec-2.c to ICE with e.g.
-march=nehalem:
markus@x4 gcc % gcc -std=c11 -pedantic-errors -march=nehalem -O2 ./gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-2.c
./gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-2.c: In function ‘test_minus’:
./gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-2.c:120:1: internal compiler error: in ix86_cc_mode, at config/i386/i386.c:22485
}
^
0xea7b37 ix86_cc_mode(rtx_code, rtx_def*, rtx_def*)
/home/markus/gcc/gcc/config/i386/i386.c:22485
0x1246e11 maybe_select_cc_mode
/home/markus/gcc/gcc/compare-elim.c:500
0x1246e11 try_eliminate_compare
/home/markus/gcc/gcc/compare-elim.c:665
0x1246e11 execute_compare_elim_after_reload
/home/markus/gcc/gcc/compare-elim.c:727
0x1246e11 execute
/home/markus/gcc/gcc/compare-elim.c:770
--
Markus