https://gcc.gnu.org/g:a01b40c047334c5f7cf69233ac6f3bfeacc24c5d
commit r15-2024-ga01b40c047334c5f7cf69233ac6f3bfeacc24c5d
Author: Hans-Peter Nilsson <h...@axis.com>
Date: Wed Jul 10 04:57:07 2024 +0200
CRIS: Disable late-combine by default, related PR115883
With late-combine, performance for coremark compiled for cris-elf
regresses 2.6% by performance and by size 0.4%, measured at
r15-2005-g13757e50ff0b, when compiled with "-O2 -march=v10".
Earlier, at r15-1880-gce34fcc572a0, numbers were by performance 3.2%
and by size 0.4%, even with the proposed patch to PR115883 (TL;DR: a
presumed bug in LRA or combine exposed by late-combine). Without that
patch, about the same performance results (at that revision).
Similarly around the late-combine commit (r15-1579-g792f97b44ffc5e).
I briefly looked at the performance regression for coremark at
r15-2005-g13757e50ff0b (with/without this patch) as far as seeing that
the stack-frame grew larger (maxing out on hard registers and needing
one more slot) for at least two of the top three* functions that
regressed the most in terms of cycles per call:
matrix_mul_matrix_bitextract (in coremark, 17% slower) and __subdf3
(in libgcc, 6.7% slower). That makes sense when considering that
late-combine "naturally" stretches register life-times. But, looking
at late_combine::combine_into_uses and late_combine::optimizable_set,
nothing stood out to me. I guess there's improvement opportunities in
late_combine::check_register_pressure.
(*) I opted not to look at _dtoa_r (in newlib) mostly because it's
boring and always shows up when something in gcc goes sideways. (It
maxes out on hard registers and is big. End of story.)
Note that the change of default is done in the
TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE worker, not in the
TARGET_OPTION_OVERRIDE worker for reasons stated in the
comment.
* config/cris/cris.cc (cris_option_override_after_change): New
function. Disable late-combine by default.
(cris_option_override): Call the new function.
Diff:
---
gcc/config/cris/cris.cc | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/gcc/config/cris/cris.cc b/gcc/config/cris/cris.cc
index f1accc0f36e5..f2c3955b9665 100644
--- a/gcc/config/cris/cris.cc
+++ b/gcc/config/cris/cris.cc
@@ -53,6 +53,7 @@ along with GCC; see the file COPYING3. If not see
#include "builtins.h"
#include "cfgrtl.h"
#include "tree-pass.h"
+#include "opts.h"
/* This file should be included last. */
#include "target-def.h"
@@ -156,6 +157,7 @@ static rtx_insn *cris_md_asm_adjust (vec<rtx> &, vec<rtx> &,
HARD_REG_SET &, location_t);
static void cris_option_override (void);
+static void cris_option_override_after_change ();
static bool cris_frame_pointer_required (void);
@@ -281,6 +283,8 @@ int cris_cpu_version = CRIS_DEFAULT_CPU_VERSION;
#undef TARGET_OPTION_OVERRIDE
#define TARGET_OPTION_OVERRIDE cris_option_override
+#undef TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE
+#define TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE cris_option_override_after_change
#undef TARGET_ASM_TRAMPOLINE_TEMPLATE
#define TARGET_ASM_TRAMPOLINE_TEMPLATE cris_asm_trampoline_template
@@ -2409,6 +2413,39 @@ cris_option_override (void)
/* Set the per-function-data initializer. */
init_machine_status = cris_init_machine_status;
+
+ cris_option_override_after_change ();
+}
+
+/* The TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE worker.
+
+ The CRIS port doesn't have any port-specific function attributes to
+ handle, but to keep attributes consistent across per-function changes
+ and not fail per-function optimization settings as exposed by
+ gcc.dg/ipa/iinline-attr.c, any OPTION_SET_P work need to be done
+ here, not in the TARGET_OPTION_OVERRIDE function. This function then
+ instead needs to called from that function. */
+
+static void
+cris_option_override_after_change ()
+{
+ /* The combine pass inserts extra copies of the incoming parameter
+ registers in make_more_copies, between the hard registers and
+ pseudo-registers holding the "original" copies. When doing that,
+ it does not copy attributes from those original registers. With
+ the late-combine pass, those extra copies are propagated into more
+ places than the original copies, and trips up LRA which doesn't see
+ e.g. REG_POINTER where it's expected. This causes an ICE for
+ gcc.target/cris/rld-legit1.c. That's a red flag, but also a very
+ special corner case.
+
+ A more valid reason is that coremark with -march=v10 -O2 regresses
+ by XXXXX% (@r15-gXXXXXXXXXXXX vs. @r15-gXXXXXXXXXXXX and also at
+ r15-gXXXXXXXXXXXX with patches to handle the REG_POINTER and
+ rld-legit1 fallout vs. this additional change). Disable
+ late-combine by default until that's fixed. */
+ if (!OPTION_SET_P (flag_late_combine_instructions))
+ flag_late_combine_instructions = 0;
}
/* The TARGET_ASM_OUTPUT_MI_THUNK worker. */
