[PATCH] LRA: Fix incorrect register spill/reload
Hello, When investigating regression with LRA enabled for mips16 I found incorrect spilling and reload of registers by callee. In the case, one register was not saved, although used, and another one never used but saved/restored. The issue appears to be in setting registers ever lived and subsequent passes save/restore the wrong register(s). I have attached a patch below. I presume that the statement terminator was typed accidentally as I do not see a justification of the df_set_regs_ever_live() function to be outside the for loop. Or I am wrong? Regards, Robert * lra-spills.c (assign_spill_hard_regs): Removed statement terminator after comment. Loop body outside the for loop. diff --git a/gcc/lra-spills.c b/gcc/lra-spills.c index 7c0c630..e1cf654 100644 --- a/gcc/lra-spills.c +++ b/gcc/lra-spills.c @@ -334,8 +334,8 @@ assign_spill_hard_regs (int *pseudo_regnos, int n) for (nr = 0; nr < hard_regno_nregs[hard_regno][lra_reg_info[regno].biggest_mode]; nr++) - /* Just loop. */; - df_set_regs_ever_live (hard_regno + nr, true); + /* Just loop. */ + df_set_regs_ever_live (hard_regno + nr, true); } bitmap_clear (&ok_insn_bitmap); free (reserved_hard_regs);
RE: [PATCH] LRA: Fix incorrect register spill/reload
Hi David, No, I do not have read/write SVN access. I know a person who could commit the patch for me, however, if you can commit it, I'd be grateful. Regards, Robert > Vladimir Makarov wrote: > Robert, thanks for finding it and informing. You can commit the patch > into the trunk. Robert, Do you have GCC SVN access? If not, please ask one of us to commit your patch for you. Thanks, David
RE: [PATCH, MIPS] Support new interrupt handler options
Hi Catherine,
> I'm getting build errors with the current TOT and your patch.
>
> The first errors that I encounter are:
> gcc/config/mips/mips.c:1355:1: warning: 'mips_int_mask
> mips_interrupt_mask(tree)' defined but not used [-Wunused-function]
> gcc/config/mips/mips.c:1392:1: warning: 'mips_shadow_set
> mips_use_shadow_register_set(tree)' defined but not used [-Wunused-function]
>
> Removing these two functions results in further errors that I have not
> investigated.
> Will you try applying and building your patch again?
I have no explanation why this could happen. My guess is that a part of the
patch
did not apply correctly. Those functions are used in mips_compute_frame_info().
I did notice and fixed warnings about unused variables/arguments.
>
> I have a couple of further comments on the existing patch, see below.
Comments added. Please have a look at the attached revised patch.
Tested against r225768.
Regards,
Robert
gcc/
* config/mips/mips.c (mips_int_mask): New enum.
(mips_shadow_set): Likewise.
(int_mask): New variable.
(use_shadow_register_set_p): Change type to enum mips_shadow_set.
(machine_function): Add int_mask and use_shadow_register_set.
(mips_attribute_table): Add attribute handlers for interrupt and
use_shadow_register_set.
(mips_interrupt_mask): New static function.
(mips_handle_interrupt_attr): Likewise.
(mips_handle_use_shadow_register_set_attr): Likewise.
(mips_use_shadow_register_set): Change return type to enum
mips_shadow_set. Add argument handling for use_shadow_register_set
attribute.
(mips_interrupt_extra_called_saved_reg_p): Update the conditional to
compare with mips_shadow_set enum.
(mips_compute_frame_info): Add interrupt mask and
use_shadow_register_set to per-function information structure.
Add a stack slot for EPC unconditionally.
(mips_expand_prologue): Compare use_shadow_register_set value
with mips_shadow_set enum. Save EPC always in K1, clobber only K1 for
masked interrupt register but in EIC mode use K0 and save Cause in K0.
EPC saved and restored unconditionally. Use PMODE_INSN macro when
copying the stack pointer from the shadow register set.
* config/mips/mips.h (SR_IM0): New define.
* config/mips/mips.md (mips_rdpgpr): Rename to...
(mips_rdpgpr_): ...this. Use the Pmode iterator.
* doc/extend.texi (Declaring Attributes of Functions): Document
optional arguments for interrupt and use_shadow_register_set
attributes.
gcc/testsuite/
* gcc.target/mips/interrupt_handler-4.c: New test.
---
gcc/config/mips/mips.c | 286 +
gcc/config/mips/mips.h | 3 +
gcc/config/mips/mips.md| 10 +-
gcc/doc/extend.texi| 22 +-
.../gcc.target/mips/interrupt_handler-4.c | 31 +++
5 files changed, 290 insertions(+), 62 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/mips/interrupt_handler-4.c
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 671fed8..70240f7 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -382,6 +382,30 @@ struct GTY(()) mips_frame_info {
HOST_WIDE_INT hard_frame_pointer_offset;
};
+/* Enumeration for masked vectored (VI) and non-masked (EIC) interrupts. */
+enum mips_int_mask
+{
+ INT_MASK_EIC = -1,
+ INT_MASK_SW0 = 0,
+ INT_MASK_SW1 = 1,
+ INT_MASK_HW0 = 2,
+ INT_MASK_HW1 = 3,
+ INT_MASK_HW2 = 4,
+ INT_MASK_HW3 = 5,
+ INT_MASK_HW4 = 6,
+ INT_MASK_HW5 = 7
+};
+
+/* Enumeration to mark the existence of the shadow register set.
+ SHADOW_SET_INTSTACK indicates a shadow register set with a valid stack
+ pointer. */
+enum mips_shadow_set
+{
+ SHADOW_SET_NO,
+ SHADOW_SET_YES,
+ SHADOW_SET_INTSTACK
+};
+
struct GTY(()) machine_function {
/* The next floating-point condition-code register to allocate
for ISA_HAS_8CC targets, relative to ST_REG_FIRST. */
@@ -434,8 +458,12 @@ struct GTY(()) machine_function {
/* True if this is an interrupt handler. */
bool interrupt_handler_p;
- /* True if this is an interrupt handler that uses shadow registers. */
- bool use_shadow_register_set_p;
+ /* Records the way in which interrupts should be masked. Only used if
+ interrupts are not kept masked. */
+ enum mips_int_mask int_mask;
+
+ /* Records if this is an interrupt handler that uses shadow registers. */
+ enum mips_shadow_set use_shadow_register_set;
/* True if this is an interrupt handler that should keep interrupts
masked. */
@@ -717,6 +745,10 @@ const enum reg_class
mips_regno_to_class[FIRST_PSEUDO_REGISTER] = {
ALL_REGS,ALL_REGS, ALL_REGS, ALL_REGS
};
+static tree mips_handle_interrupt_attr (tree *, tree, tree, int, bool *);
+static tree
RE: [PATCH, MIPS] Fix restoration of hi/lo in MIPS64R2 interrupt handlers
Hi Catherine,
> Hi Robert,
> The patch is OK, but will you please name the test something other than the
> date?
OK. I'll change it to interrupt_handler-5.c, add a comment and commit after
approval for the new interrupt handler options.
Regards,
Robert
diff --git a/gcc/testsuite/gcc.target/mips/interrupt_handler-5.c
b/gcc/testsuite/gcc.target/mips/interrupt_handler-5.c
new file mode 100644
index 000..6419479
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/interrupt_handler-5.c
@@ -0,0 +1,8 @@
+/* Test the interrupt handler with an accumulator. */
+/* { dg-do assemble } */
+/* { dg-options "-mips64r2" } */
+_Accum a;
+__attribute__((interrupt))
+void foo () {
+ a = a*a;
+}
--
2.2.2
RE: [PATCH, MIPS] Support interrupt handlers with hard-float
Hi, > > Hi Matthew/Catherine, > > > > The attached patch removes the restriction to compile a TU with an ISR with > - > > mhard-float. Instead of forcing -msoft-float, the coprocessor 1 is disabled > in > > an ISR for -mhard-float. > > > > Ok to apply? > > Yes, this one is OK. Committed as r225818. Regards, Robert
RE: [PATCH, MIPS] Support new interrupt handler options
Hi Catherine, > This is now OK to commit. > Catherine Committed as r225819. Robert
RE: [PATCH, MIPS] Fix restoration of hi/lo in MIPS64R2 interrupt handlers
Hi, > > OK. I'll change it to interrupt_handler-5.c, add a comment and commit after > > approval for the new interrupt handler options. > > I believe this change is independent of the new attributes so feel free to > commit > it before. I was to going to commit it before but by the time I did that, I got approval for all patches and committed in order. Committed as r225820. Regards, Robert
[PATCH, MIPS] Add -march=interaptiv
Hi,
As in the title, the attached patch adds -march=interaptiv defined to 24kf2_1,
mapped to -mips32r2 and -mdsp.
OK to apply?
Regards,
Robert
gcc/
* config/mips/mips-cpus.def (interaptiv): Define.
* config/mips/mips-tables.opt: Regenerate.
* config/mips/mips.h (MIPS_ISA_LEVEL_SPEC): Map -march=interaptiv to
-mips32r2.
(BASE_DRIVER_SELF_SPECS): Likewise but map to -mdsp.
* doc/invoke.texi (-march=@var{arch}): Add interaptiv.
---
gcc/config/mips/mips-cpus.def | 2 ++
gcc/config/mips/mips-tables.opt | 39 +--
gcc/config/mips/mips.h | 6 --
gcc/doc/invoke.texi | 1 +
4 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/gcc/config/mips/mips-cpus.def b/gcc/config/mips/mips-cpus.def
index fb4bae0..63a0d6e 100644
--- a/gcc/config/mips/mips-cpus.def
+++ b/gcc/config/mips/mips-cpus.def
@@ -147,6 +147,8 @@ MIPS_CPU ("1004kf2_1", PROCESSOR_24KF2_1, 33, 0)
MIPS_CPU ("1004kf", PROCESSOR_24KF2_1, 33, 0)
MIPS_CPU ("1004kf1_1", PROCESSOR_24KF1_1, 33, 0)
+MIPS_CPU ("interaptiv", PROCESSOR_24KF2_1, 33, 0)
+
/* MIPS32 Release 5 processors. */
MIPS_CPU ("p5600", PROCESSOR_P5600, 36, PTF_AVOID_BRANCHLIKELY)
diff --git a/gcc/config/mips/mips-tables.opt b/gcc/config/mips/mips-tables.opt
index 59124a6..8c6c4b1 100644
--- a/gcc/config/mips/mips-tables.opt
+++ b/gcc/config/mips/mips-tables.opt
@@ -631,56 +631,59 @@ EnumValue
Enum(mips_arch_opt_value) String(r1004kf1_1) Value(84)
EnumValue
-Enum(mips_arch_opt_value) String(p5600) Value(85) Canonical
+Enum(mips_arch_opt_value) String(interaptiv) Value(85) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(5kc) Value(86) Canonical
+Enum(mips_arch_opt_value) String(p5600) Value(86) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(r5kc) Value(86)
+Enum(mips_arch_opt_value) String(5kc) Value(87) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(5kf) Value(87) Canonical
+Enum(mips_arch_opt_value) String(r5kc) Value(87)
EnumValue
-Enum(mips_arch_opt_value) String(r5kf) Value(87)
+Enum(mips_arch_opt_value) String(5kf) Value(88) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(20kc) Value(88) Canonical
+Enum(mips_arch_opt_value) String(r5kf) Value(88)
EnumValue
-Enum(mips_arch_opt_value) String(r20kc) Value(88)
+Enum(mips_arch_opt_value) String(20kc) Value(89) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(sb1) Value(89) Canonical
+Enum(mips_arch_opt_value) String(r20kc) Value(89)
EnumValue
-Enum(mips_arch_opt_value) String(sb1a) Value(90) Canonical
+Enum(mips_arch_opt_value) String(sb1) Value(90) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(sr71000) Value(91) Canonical
+Enum(mips_arch_opt_value) String(sb1a) Value(91) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(sr71k) Value(91)
+Enum(mips_arch_opt_value) String(sr71000) Value(92) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(xlr) Value(92) Canonical
+Enum(mips_arch_opt_value) String(sr71k) Value(92)
EnumValue
-Enum(mips_arch_opt_value) String(loongson3a) Value(93) Canonical
+Enum(mips_arch_opt_value) String(xlr) Value(93) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(octeon) Value(94) Canonical
+Enum(mips_arch_opt_value) String(loongson3a) Value(94) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(octeon+) Value(95) Canonical
+Enum(mips_arch_opt_value) String(octeon) Value(95) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(octeon2) Value(96) Canonical
+Enum(mips_arch_opt_value) String(octeon+) Value(96) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(octeon3) Value(97) Canonical
+Enum(mips_arch_opt_value) String(octeon2) Value(97) Canonical
EnumValue
-Enum(mips_arch_opt_value) String(xlp) Value(98) Canonical
+Enum(mips_arch_opt_value) String(octeon3) Value(98) Canonical
+
+EnumValue
+Enum(mips_arch_opt_value) String(xlp) Value(99) Canonical
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 37f5b54..505e111 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -722,7 +722,8 @@ struct mips_cpu_info {
|march=r1|march=r12000|march=r14000|march=r16000:-mips4} \
%{march=mips32|march=4kc|march=4km|march=4kp|march=4ksc:-mips32} \
%{march=mips32r2|march=m4k|march=4ke*|march=4ksd|march=24k* \
- |march=34k*|march=74k*|march=m14k*|march=1004k*: -mips32r2} \
+ |march=34k*|march=74k*|march=m14k*|march=1004k* \
+ |march=interaptiv: -mips32r2} \
%{march=mips32r3: -mips32r3} \
%{march=mips32r5|march=p5600: -mips32r5} \
%{march=mips32r6: -mips32r6} \
@@ -825,7 +826,8 @@ struct mips_cpu_info {
#define BASE_DRIVER_SELF_SPECS \
MIPS_ISA_NAN2008_SPEC, \
"%{!mno-dsp: \
- %{march=24ke*|march=34kc*|march=34kf*|march=34kx*|march=1004k*: -mdsp} \
+ %{march=24ke*|march=34kc*|march=34kf*|march=34kx*|march=1004k* \
+ |march=interaptiv: -mdsp} \
%{march=74k*|march=m14ke*: %{!mno-
[PATCH, MIPS] I6400 scheduling
Hi,
This patch adds a pipeline description for the I6400 processor with -mips32r6
and -mips64r6 defaulted to this description.
Regtested with mips-img-linux-gnu. mips-tables.opt will be regenerated before
committing depending on which patch from the series goes in first.
Ok to apply?
Regards,
Robert
2015-07-16 Prachi Godbole
gcc/
* config/mips/i6400.md: New file.
* config/mips/mips-cpus.def (mips32r6): Change to PROCESSOR_I6400.
(mips64r6): Likewise.
(i6400): Define.
* config/mips/mips-tables.opt: Regenerate.
* config/mips/mips.c (mips_rtx_cost_data): Add I6400 processor.
(mips_issue_rate): Add support for i6400.
(mips_multipass_dfa_lookahead): Likewise.
* config/mips/mips.h (TUNE_I6400): Define.
* config/mips/mips.md: Include i6400.md.
(processor): Add i6400.
* doc/invoke.texi (-march=@var{arch}): Add i6400.
---
gcc/config/mips/i6400.md| 142
gcc/config/mips/mips-cpus.def | 7 +-
gcc/config/mips/mips-tables.opt | 3 +
gcc/config/mips/mips.c | 16 -
gcc/config/mips/mips.h | 3 +-
gcc/config/mips/mips.md | 2 +
gcc/doc/invoke.texi | 1 +
7 files changed, 170 insertions(+), 4 deletions(-)
create mode 100644 gcc/config/mips/i6400.md
diff --git a/gcc/config/mips/i6400.md b/gcc/config/mips/i6400.md
new file mode 100644
index 000..101a20c
--- /dev/null
+++ b/gcc/config/mips/i6400.md
@@ -0,0 +1,142 @@
+;; DFA-based pipeline description for I6400.
+;;
+;; Copyright (C) 2007-2015 Free Software Foundation, Inc.
+;;
+;; This file is part of GCC.
+;;
+;; GCC is free software; you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published
+;; by the Free Software Foundation; either version 3, or (at your
+;; option) any later version.
+
+;; GCC is distributed in the hope that it will be useful, but WITHOUT
+;; ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+;; or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
+;; License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GCC; see the file COPYING3. If not see
+;;
[PATCH, MIPS] Scheduling for M51xx core family
Hi,
Another patch with a pipeline description but for M51xx cores with
two new options introduced: -march={m5100,m5101}. The M5101 is essentially
the same as M5100 but mapped to -msoft-float.
Ok to apply?
Regards,
Robert
2015-07-16 Prachi Godbole
gcc/
* config/mips/m5100.md: New file.
* config/mips/mips-cpus.def (m5100, m5101): Define.
* config/mips/mips-tables.opt: Regenerate.
* config/mips/mips.c (mips_rtx_cost_data): Add costs for m5100.
* config/mips/mips.h (MIPS_ISA_LEVEL_SPEC): Map -march=m5100 and
-march=m5101 to -mips32r5.
(MIPS_ARCH_FLOAT_SPEC): Map -m5101 to -msoft-float.
(MIPS_ISA_NAN2008_SPEC): Map -march=m51* to -mnan=2008 if
!-msoft-float.
* config/mips/mips.md: Include m5100.md.
(processor): Add m5100.
* doc/invoke.texi (-march=@var{arch}): Add m5100, m5101.
---
gcc/config/mips/m5100.md| 220
gcc/config/mips/mips-cpus.def | 2 +
gcc/config/mips/mips-tables.opt | 40
gcc/config/mips/mips.c | 13 +++
gcc/config/mips/mips.h | 7 +-
gcc/config/mips/mips.md | 2 +
gcc/doc/invoke.texi | 1 +
7 files changed, 265 insertions(+), 20 deletions(-)
create mode 100644 gcc/config/mips/m5100.md
diff --git a/gcc/config/mips/m5100.md b/gcc/config/mips/m5100.md
new file mode 100644
index 000..f860eb2
--- /dev/null
+++ b/gcc/config/mips/m5100.md
@@ -0,0 +1,220 @@
+;; DFA-based pipeline description for MIPS32 models M5100.
+;;
+;; Copyright (C) 2015 Free Software Foundation, Inc.
+;;
+;; This file is part of GCC.
+;;
+;; GCC is free software; you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published
+;; by the Free Software Foundation; either version 3, or (at your
+;; option) any later version.
+
+;; GCC is distributed in the hope that it will be useful, but WITHOUT
+;; ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+;; or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
+;; License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GCC; see the file COPYING3. If not see
+;;
RE: [PATCH, MIPS] Scheduling for M51xx core family
Hi Matthew,
> > gcc/
> >
> > * config/mips/m5100.md: New file.
> > * config/mips/mips-cpus.def (m5100, m5101): Define.
> > * config/mips/mips-tables.opt: Regenerate.
> > * config/mips/mips.c (mips_rtx_cost_data): Add costs for m5100.
> > * config/mips/mips.h (MIPS_ISA_LEVEL_SPEC): Map -march=m5100 and
> > -march=m5101 to -mips32r5.
> > (MIPS_ARCH_FLOAT_SPEC): Map -m5101 to -msoft-float.
> > (MIPS_ISA_NAN2008_SPEC): Map -march=m51* to -mnan=2008 if
> > !-msoft-float.
> > * config/mips/mips.md: Include m5100.md.
> > (processor): Add m5100.
> > * doc/invoke.texi (-march=@var{arch}): Add m5100, m5101.
>
> OK, this looks fine.
The patch committed as r226065.
> I did realise while reading through this that the MIPS_ARCH_FLOAT_SPEC
> is not used for and ordinary MIPS Linux compiler which seems odd but
> I presume this is to make it possible to use one hard-float sysroot
> for any core and emulate the FPU when not present.
>
> I think it is probably a mistake to have put MIPS_ARCH_FLOAT_SPEC in
> the mti-linux.h and android.h DRIVER_SELF_SPECS so I think they need
> removing. Although we support building soft-float multilibs I don't
> think they actually get used very much so leaving the selection of
> soft-float down to the end user in Linux seems wise.
>
> With the i6400 scheduler committed then we can also get rid of the w32
> and w64 placeholders that were there solely to provide an R6 processor
> to use as the default processor for the generic arch options.
I'll send a follow-up patch to remove the redundant bits.
Regards,
Robert
RE: [PATCH, MIPS] Add -march=interaptiv
Hi Catherine,
> > gcc/
> > * config/mips/mips-cpus.def (interaptiv): Define.
> > * config/mips/mips-tables.opt: Regenerate.
> > * config/mips/mips.h (MIPS_ISA_LEVEL_SPEC): Map -
> > march=interaptiv to
> > -mips32r2.
> > (BASE_DRIVER_SELF_SPECS): Likewise but map to -mdsp.
> > * doc/invoke.texi (-march=@var{arch}): Add interaptiv.
> > ---
>
> Yes, this looks OK.
Committed as r226064.
Regards,
Robert
RE: [PATCH, MIPS] I6400 scheduling
Hi,
> > diff --git a/gcc/config/mips/i6400.md b/gcc/config/mips/i6400.md new
> > file mode 100644 index 000..101a20c
> > --- /dev/null
> > +++ b/gcc/config/mips/i6400.md
> > @@ -0,0 +1,142 @@
> > +;; DFA-based pipeline description for I6400.
> > +;;
> > +;; Copyright (C) 2007-2015 Free Software Foundation, Inc.
>
> This should just be 2015.
Fixed.
> > diff --git a/gcc/config/mips/mips-cpus.def b/gcc/config/mips/mips-
> > cpus.def index fb4bae0..90836a3 100644
> > --- a/gcc/config/mips/mips-cpus.def
> > +++ b/gcc/config/mips/mips-cpus.def
> > @@ -50,13 +50,13 @@ MIPS_CPU ("mips32r2", PROCESSOR_74KF2_1, 33,
> > PTF_AVOID_BRANCHLIKELY)
> > as mips32r2. */
> > MIPS_CPU ("mips32r3", PROCESSOR_M4K, 34, PTF_AVOID_BRANCHLIKELY)
> > MIPS_CPU ("mips32r5", PROCESSOR_P5600, 36, PTF_AVOID_BRANCHLIKELY) -
> > MIPS_CPU ("mips32r6", PROCESSOR_W32, 37, PTF_AVOID_BRANCHLIKELY)
> > +MIPS_CPU ("mips32r6", PROCESSOR_I6400, 37, PTF_AVOID_BRANCHLIKELY)
> > MIPS_CPU ("mips64", PROCESSOR_5KC, 64, PTF_AVOID_BRANCHLIKELY)
> > /* ??? For now just tune the generic MIPS64r2 and above for 5KC as
> > well. */
> > MIPS_CPU ("mips64r2", PROCESSOR_5KC, 65, PTF_AVOID_BRANCHLIKELY)
> > MIPS_CPU ("mips64r3", PROCESSOR_5KC, 66, PTF_AVOID_BRANCHLIKELY)
> > MIPS_CPU ("mips64r5", PROCESSOR_5KC, 68, PTF_AVOID_BRANCHLIKELY) -
> > MIPS_CPU ("mips64r6", PROCESSOR_W64, 69, PTF_AVOID_BRANCHLIKELY)
> > +MIPS_CPU ("mips64r6", PROCESSOR_I6400, 69, PTF_AVOID_BRANCHLIKELY)
> >
> > /* MIPS I processors. */
> > MIPS_CPU ("r3000", PROCESSOR_R3000, 1, 0) @@ -166,3 +166,6 @@ MIPS_CPU
> > ("octeon+", PROCESSOR_OCTEON, 65, PTF_AVOID_BRANCHLIKELY) MIPS_CPU
> > ("octeon2", PROCESSOR_OCTEON2, 65, PTF_AVOID_BRANCHLIKELY) MIPS_CPU
> > ("octeon3", PROCESSOR_OCTEON3, 65, PTF_AVOID_BRANCHLIKELY) MIPS_CPU
> > ("xlp", PROCESSOR_XLP, 65, PTF_AVOID_BRANCHLIKELY)
> > +
> > +/* MIPS64 Release 6 processors. */
> > +MIPS_CPU ("i6400", PROCESSOR_I6400, 69, PTF_AVOID_BRANCHLIKELY)
>
> I don't think this really matters but the PTF_AVOID_BRANCHLIKELY should
> not be necessary for R6 cores as there are no branch likely instructions.
> Changing this may also require an update to the option handling code
> in mips.c I don't know if it will try to enable branch likely if you
> remove this.
PTF_AVOID_BRANCHLIKELY replaced with 0 in all 3 cases.
AFAICS, there is no need to update the option handling code. The branch
likely will not be enabled as it is additionally guarded by
ISA_HAS_BRANCHLIKELY.
>
> OK with those changes.
I'll commit the updated patch once the build completes.
> Does the I6400 support load/store bonding? I seem to think it does but
> could be wrong. If it does then dealing with it in a follow up patch is
> OK with me.
It does support the load/store bonding. I'll test and send another patch
for this.
Regards,
Robert
RE: [PATCH, MIPS] I6400 scheduling
Hi, > PTF_AVOID_BRANCHLIKELY replaced with 0 in all 3 cases. > AFAICS, there is no need to update the option handling code. The branch > likely will not be enabled as it is additionally guarded by > ISA_HAS_BRANCHLIKELY. > > > > > OK with those changes. > > I'll commit the updated patch once the build completes. Committed as r226090. Regards, Robert
[PATCH, MIPS] Enable load/store bonding for I6400
Hi, Following up https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01730.html The patch below enables the load-load/store-store bonding for MIPS32/MIPS64 R6. Ok to apply? Regards, Robert gcc/ * config/mips/mips.h (ENABLE_LD_ST_PAIRS): Enable load/store pairs for I6400. --- gcc/config/mips/mips.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index d17a833..6e262d6 100644 --- a/gcc/config/mips/mips.h +++ b/gcc/config/mips/mips.h @@ -3177,5 +3177,5 @@ extern GTY(()) struct target_globals *micromips_globals; performance can be degraded for those targets. Hence, do not bond for micromips or fix_24k. */ #define ENABLE_LD_ST_PAIRS \ - (TARGET_LOAD_STORE_PAIRS && TUNE_P5600 \ + (TARGET_LOAD_STORE_PAIRS && (TUNE_P5600 || TUNE_I6400) \ && !TARGET_MICROMIPS && !TARGET_FIX_24K) -- 2.4.5
[PATCH, MIPS] Remove W32 and W64 pseudo-processors
Hi,
Since the I6400 scheduler is committed, W32/W64 pseudo-processors
are not needed anymore and can be removed.
Ok to commit?
Regards,
Robert
gcc/
* config/mips/mips.c (mips_rtx_cost_data): Remove costs for W32 and W64
pseudo-processors.
* config/mips/mips.md (processor): Remove w32 and w64.
---
gcc/config/mips/mips.c | 26 --
gcc/config/mips/mips.md | 2 --
2 files changed, 28 deletions(-)
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index bf0f84f..b30d7b9 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -1255,32 +1255,6 @@ static const struct mips_rtx_cost_data
2,/* branch_cost */
4 /* memory_latency */
},
- { /* W32 */
-COSTS_N_INSNS (4),/* fp_add */
-COSTS_N_INSNS (4),/* fp_mult_sf */
-COSTS_N_INSNS (5),/* fp_mult_df */
-COSTS_N_INSNS (17), /* fp_div_sf */
-COSTS_N_INSNS (32), /* fp_div_df */
-COSTS_N_INSNS (5),/* int_mult_si */
-COSTS_N_INSNS (5),/* int_mult_di */
-COSTS_N_INSNS (41), /* int_div_si */
-COSTS_N_INSNS (41), /* int_div_di */
-1, /* branch_cost */
-4/* memory_latency */
- },
- { /* W64 */
-COSTS_N_INSNS (4),/* fp_add */
-COSTS_N_INSNS (4),/* fp_mult_sf */
-COSTS_N_INSNS (5),/* fp_mult_df */
-COSTS_N_INSNS (17), /* fp_div_sf */
-COSTS_N_INSNS (32), /* fp_div_df */
-COSTS_N_INSNS (5),/* int_mult_si */
-COSTS_N_INSNS (5),/* int_mult_di */
-COSTS_N_INSNS (41), /* int_div_si */
-COSTS_N_INSNS (41), /* int_div_di */
-1, /* branch_cost */
-4/* memory_latency */
- },
{ /* M5100 */
COSTS_N_INSNS (4),/* fp_add */
COSTS_N_INSNS (4),/* fp_mult_sf */
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 2954a12..a0079d5 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -67,8 +67,6 @@ (define_enum "processor" [
xlr
xlp
p5600
- w32
- w64
m5100
i6400
])
--
2.4.5
[RFC][PATCH] Preferred rename register in regrename pass
Hi,
We came across a situation for MIPS64 where moves for sign-extension were
not converted into a nop because of IRA spilled some of the allocnos and
assigned different hard register for the output operand in the move.
LRA is not fixing this up as most likely the move was not introduced by
the LRA itself. I found it hard to fix this in LRA and looked at
an alternative solution where regrename pass appeared to be the best candidate.
The patch below introduces a notion of a preferred rename register and attempts
to use the output register for an input register iff the input register dies
in an instruction. The preferred register is validated and in the case it fails
to be validated, it falls back to the old technique of finding the best rename
register.
Of course, it has slightly limited scope of use as it's not enabled be default,
however, when targeting performance one is likely to enable it via
-funroll-loops or -fpeel-loops.
I did some experiments with -funroll-loops on x86_64-unknown-linux-gnu and
the code size improved almost 0.4% on average case. I haven't done an extensive
performance testing but it appeared SPEC2006 had some minor improvement on
average,
which could be real improvement or just noise.
On MIPS64 with -funroll-loops, there were a number of cases where the
unnecessary
moves turned into a nop in CSiBE. MIPS32 also marginally improved but to
a lower degree.
The patch successfully passed x86_64-unknown-linux-gnu, mips-mti-linux-gnu and
mips-img-linux-gnu.
I'm not sure if this is something that should be enabled by default for everyone
or a target hook should be added. Any other comments/suggestions?
Regards,
Robert
gcc/
* regrename.c (find_preferred_rename_reg): New function.
(record_operand_use): Remove assertion. Allocate or resize heads and
chains vectors, if necessary.
(find_best_rename_reg): Use the new function and validate chosen
register.
(build_def_use): Don't allocate and initialize space of size 0.
(regrename_finish): Free heads and chains vectors.
(regrename_optimize): Pass true to initializing function.
* regrename.h (struct operand_rr_info): Replace arrays of heads and
chains with vectors.
---
gcc/regrename.c | 86 +
gcc/regrename.h | 4 +--
2 files changed, 82 insertions(+), 8 deletions(-)
diff --git a/gcc/regrename.c b/gcc/regrename.c
index c328c1b..90fee98 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -174,6 +174,51 @@ dump_def_use_chain (int from)
}
}
+/* Return a preferred rename register for HEAD. */
+
+static int
+find_preferred_rename_reg (du_head_p head)
+{
+ struct du_chain *this_du;
+ int preferred_reg = -1;
+
+ for (this_du = head->first; this_du; this_du = this_du->next_use)
+{
+ rtx note;
+ insn_rr_info *p;
+
+ /* The preferred rename register is an output register iff an input
+register dies in an instruction but the candidate must be validated by
+check_new_reg_p. */
+ for (note = REG_NOTES (this_du->insn); note; note = XEXP (note, 1))
+ if (insn_rr.exists()
+ && REG_NOTE_KIND (note) == REG_DEAD
+ && REGNO (XEXP (note, 0)) == head->regno
+ && (p = &insn_rr[INSN_UID (this_du->insn)])
+ && p->op_info)
+ {
+ int i;
+ for (i = 0; i < p->op_info->n_chains; i++)
+ {
+ struct du_head *next_head = p->op_info->heads[i];
+ if (head != next_head)
+ {
+ preferred_reg = next_head->regno;
+ if (dump_file)
+ fprintf (dump_file,
+ "Chain %s (%d) has preferred rename register"
+ " %s for insn %d [%s]\n",
+ reg_names[head->regno], head->id,
+ reg_names[preferred_reg],
+ INSN_UID (this_du->insn),
+ reg_class_names[this_du->cl]);
+ }
+ }
+ }
+}
+ return preferred_reg;
+}
+
static void
free_chain_data (void)
{
@@ -206,7 +251,16 @@ record_operand_use (struct du_head *head, struct du_chain
*this_du)
{
if (cur_operand == NULL)
return;
- gcc_assert (cur_operand->n_chains < MAX_REGS_PER_ADDRESS);
+
+ if (!cur_operand->heads.exists ())
+cur_operand->heads.create (0);
+ if (!cur_operand->chains.exists ())
+cur_operand->chains.create (0);
+ if (cur_operand->heads.length () <= (unsigned) cur_operand->n_chains)
+cur_operand->heads.safe_grow_cleared (cur_operand->n_chains + 1);
+ if (cur_operand->chains.length () <= (unsigned) cur_operand->n_chains)
+cur_operand->chains.safe_grow_cleared (cur_operand->n_chains + 1);
+
cur_operand->heads[cur_operand->n_chains] = head;
cur_operand->chains[cur_operand->n_chains++] = this_du;
}
@@ -355,6 +409,7 @@ f
RE: [RFC][PATCH] Preferred rename register in regrename pass
Hi Bernd,
Thanks for the comments, much appreciated. Comments inlined and a reworked
patch attached.
> On 09/17/2015 04:38 PM, Robert Suchanek wrote:
> > We came across a situation for MIPS64 where moves for sign-extension were
> > not converted into a nop because of IRA spilled some of the allocnos and
> > assigned different hard register for the output operand in the move.
> > LRA is not fixing this up as most likely the move was not introduced by
> > the LRA itself. I found it hard to fix this in LRA and looked at
> > an alternative solution where regrename pass appeared to be the best
> candidate.
>
> For reference, please post examples of the insn pattern(s) where you
> would hope to get an improvement. Do they use matching constraints
> between the input and output operands in at least one alternative?
It all started because of 'extendmn2' SPN on MIPS64 where IRA broke the tie
between input and output registers causing the move to stay around. Generally,
I was expecting some of the moves to disappear. Initially I thought
that the approach would benefit for matching constraints but whilst reworking
and more detailed analysis I couldn't find a case.
> So this does look like something that could be addressed in regrename,
> but I think the patch is not quite the way to do it.
>
> > +/* Return a preferred rename register for HEAD. */
>
> Function comments ideally ought to be a little more detailed. Preferred
> how and why?
Noted. Will provide better description in the future.
>
> > +static int
> > +find_preferred_rename_reg (du_head_p head)
> > +{
> > + struct du_chain *this_du;
> > + int preferred_reg = -1;
> > +
> > + for (this_du = head->first; this_du; this_du = this_du->next_use)
>
> This loop seems to search for the insn where the chain terminates (i.e.
> the register dies). It seems strange to do this here rather than during
> the initial scan in record_out_operands where we visit every insn and
> already look for REG_DEAD notes.
At the time, I thought it seemed to ok to do this separately.
>
> > + rtx note;
> > + insn_rr_info *p;
> > +
> > + /* The preferred rename register is an output register iff an input
> > +register dies in an instruction but the candidate must be validated by
> > +check_new_reg_p. */
> > + for (note = REG_NOTES (this_du->insn); note; note = XEXP (note, 1))
> > + if (insn_rr.exists()
> > + && REG_NOTE_KIND (note) == REG_DEAD
> > + && REGNO (XEXP (note, 0)) == head->regno
> > + && (p = &insn_rr[INSN_UID (this_du->insn)])
> > + && p->op_info)
> > + {
> > + int i;
> > + for (i = 0; i < p->op_info->n_chains; i++)
> > + {
> > + struct du_head *next_head = p->op_info->heads[i];
> > + if (head != next_head)
>
> Here you're not actually verifying the chosen preferred reg is an
> output? Is the use of plain "p->op_info" (which is actually an array)
> intentional as a guess that operand 0 is the output? I'm not thrilled
> with this, and at the very least it should be "p->op_info[0]." to avoid
> reader confusion.
> It's also not verifying that this is indeed a case where choosing a
> preferred reg has a beneficial effect at all.
I realized that the check done in find_rename_reg should have been moved here
as,
indeed, it is rather confusing. This is more like finding a candidate rather
than validating it.
AFAICS, "p->op_info" is a pointer to struct operand_rr_info and it can be null
if a chain is opened but not used in a BB. It appears this happens if
a register is live across BB but not used in the currently processed one.
>
> The use of insn_rr would probably also be unnecessary if this was done
> during the scan phase.
>
> > + preferred_reg = next_head->regno;
>
> The problem here is that there's an ordering issue. What if next_head
> gets renamed afterwards? The choice of preferred register hasn't bought
> us anything in that case.
>
> For all these reasons I'd suggest a different approach, looking for such
> situations during the scan. Try to detect a situation where
> * we have a REG_DEAD note for an existing chain
> * the insn fulfils certain conditions (i.e. it's a move, or maybe one
> of the alternatives has a matching constraint). After all, there's
> not much point in tying if the reg that dies was used in a memory
> address.
> * a new chain is started for a single output
> Then, instead of picking a best register, mark t
RE: [RFC][PATCH] Preferred rename register in regrename pass
Hi Bernd,
> Hi Robert,
> > gcc/
> > * regrename.c (create_new_chain): Initialize terminated_dead,
> > renamed and tied_chain.
> > (find_best_rename_reg): Pick and check register from the tied chain.
> > (regrename_do_replace): Mark head as renamed.
> > (scan_rtx_reg): Tie chains in move insns. Set terminate_dead flag.
> > * regrename.h (struct du_head): Add tied_chain, renamed and
> > terminated_dead members.
>
> Thanks - this looks a lot better already. You didn't say how it was
> bootstrapped and tested; please include this information for future
> submissions. For a patch like this, some data on the improvement you got
> would also be appreciated.
Ah, sorry. I bootstrapped on x86_64-unknown-linux-gnu and ran the Dejagnu
with -frename-registers. All looked fine. As for the data, I'll do
the comparison and will update this thread by next week.
>
> I'd still like to investigate the possibility of further simplification:
>
> > + {
> > + /* Find the input chain. */
> > + for (i = c->id - 1; id_to_chain.iterate (i, &head); i--)
> > + if (head->last && head->last->insn == insn
> > + && head->terminated_dead)
> > + {
> > + gcc_assert (head->regno == REGNO (recog_data.operand[1]));
> > + c->tied_chain = head;
> > + head->tied_chain = c;
> > +
> > + if (dump_file)
> > + fprintf (dump_file, "Tying chain %s (%d) with %s (%d)\n",
> > + reg_names[c->regno], c->id,
> > + reg_names[head->regno], head->id);
> > + /* Once tied, we're done. */
> > + break;
> > + }
> > + }
> > + }
> > +
> This looks like it's a little more complicated than necessary. Couldn't
> you add a static var "terminated_this_insn" which gets initialized to
> NULL and set when a reg dies, and then you check this here rather than
> having a loop? That would also eliminate the new "terminated_dead" field.
That is a good idea. I'll add the changes and update together
with the results.
> Other than that I'm pretty happy with this.
>
>
> Bernd
Regards,
Robert
RE: [RFC][PATCH] Preferred rename register in regrename pass
Hi Bernd,
Sorry for late reply.
The updated patch was bootstrapped on x86_64-unknown-linux-gnu and cross tested
on mips-img-linux-gnu using r229786.
The results below were generated for CSiBE benchmark and the numbers in
columns express bytes in format 'net (gain/loss)' to show the difference
with and without the patch when -frename-registers switch is used.
I looked at the gains, especially for MIPS and 'teem', and it appears
that renaming registers affects the rtl_dce pass i.e. makes it less effective.
However, on average case the patch appears to reduce the code size
slightly and moves are genuinely removed.
I haven't tested the performance extensively but the SPEC benchmarks
showed almost the same results, which could be just the noise.
| MIPS n64 -Os | MIPS o32 -Os | x86_64 -Os |
---+++--+
bzip2-1.0.2| -32 (0/-32) | -24 (0/-24) | -34 (1/-35)|
cg_compiler| -172 (0/-172) | -156 (0/-156) | -46 (0/-46)|
compiler | -36 (0/-36) | -24 (0/-24) | -6(0/-6) |
flex-2.5.31| -68 (0/-68) | -80 (0/-80) | -98 (7/-105) |
jikespg-1.3| -284 (0/-284) | -204 (0/-204) | -127 (9/-136) |
jpeg-6b| -52 (8/-60) | -20 (0/-20) | -80 (11/-91) |
libmspack | -136 (0/-136) | -28 (0/-28) | -33 (23/-56) |
libpng-1.2.5 | -72 (0/-72) | -64 (0/-64) | -176 (14/-190) |
linux-2.4.23 | -700 (20/-720) | -384 (0/-384) | -691 (44/-735) |
lwip-0.5.3 | -4 (0/-4)| -4 (0/-4)| +4(13/-9)|
mpeg2dec-0.3.1 | -16 (0/-16) || -142 (6/-148) |
mpgcut-1.1 | -24 (0/-24) | -12 (4/-16) | -2(0/-2) |
OpenTCP-1.0.4 | -28 (0/-28) | -12 (0/-12) | -1(0/-1) |
replaypc-0.4.0 | -32 (0/-32) | -12 (0/-12) | -4(2/-6) |
teem-1.6.0 | -88 (480/-568)| +108 (564/-456)| -1272 (117/-1389)|
ttt-0.10.1 | -24 (0/-24) | -20 (0/-20) | -16 (0/-16)|
unrarlib-0.4.0 | -20 (0/-20) | -8 (0/-8)| -59 (9/-68)|
zlib-1.1.4 | -12 (0/-12) | -4 (0/-4)| -23 (8/-31)|
| MIPS n64 -O2 | MIPS o32 -O2 | x86_64 -O2 |
---+++--+
bzip2-1.0.2| -104 (0/-104) | -48 (0/-48) | -55 (0/-55)|
cg_compiler| -184 (4/-188) | -232 (0/-232) | -31 (5/-36)|
compiler | -32 (0/-32) | -12 (0/-12) | -4(1/-5) |
flex-2.5.31| -96 (0/-96) | -112 (0/-112) | -12 (34/-46) |
jikespg-1.3| -540 (20/-560) | -476 (4/-480) | -154 (30/-184) |
jpeg-6b| -112 (16/-128) | -60 (0/-60) | -136 (84/-220) |
libmspack | -164 (0/-164) | -40 (0/-40) | -87 (32/-119) |
libpng-1.2.5 | -120 (8/-128) | -92 (4/-96) | -140 (53/-193) |
linux-2.4.23 | -596 (12/-608) | -320 (8/-328) | -794 (285/-1079)|
lwip-0.5.3 | -8 (0/-8)| -8 (0/-8)| +2(4/-2) |
mpeg2dec-0.3.1 | -44 (0/-44) | -4 (0/-4)| -122 (8/-130) |
mpgcut-1.1 | -8 (0/-8)| -8 (0/-8)| +28 (32/-4)|
OpenTCP-1.0.4 | -4 (0/-4)| -4 (0/-4)| -2(0/-2) |
replaypc-0.4.0 | -20 (0/-20) | -24 (0/-24) | -13 (0/-13)|
teem-1.6.0 | +100 (740/-640)| +84 (736/-652)| -1998 (168/-2166)|
ttt-0.10.1 | -16 (0/-16) || |
unrarlib-0.4.0 | -16 (0/-16) | -8 (0/-8)| +19 (37/-18) |
zlib-1.1.4 | -12 (0/-12) | -4 (0/-4)| -15 (1/-16)|
Regards,
Robert
> Hi Robert,
> > gcc/
> > * regrename.c (create_new_chain): Initialize terminated_dead,
> > renamed and tied_chain.
> > (find_best_rename_reg): Pick and check register from the tied chain.
> > (regrename_do_replace): Mark head as renamed.
> > (scan_rtx_reg): Tie chains in move insns. Set terminate_dead flag.
> > * regrename.h (struct du_head): Add tied_chain, renamed and
> > terminated_dead members.
>
> Thanks - this looks a lot better already. You didn't say how it was
> bootstrapped and tested; please include this information for future
> submissions. For a patch like this, some data on the improvement you got
> would also be appreciated.
>
> I'd still like to investigate the possibility of further simplification:
>
> > + {
> > + /* Find the input chain. */
> > + for (i = c->id - 1; id_to_chain.iterate (i, &head); i--)
> > + if (head->last && head->last->insn == insn
> > + && head->terminated_dead)
> > + {
> > + gcc_assert (head->regno == REGNO (recog_data.operand[1]));
> > + c->tied_chain = head;
> > + head->tied_chain = c;
> > +
> > + if (dump_file)
> > + fprintf (dump_file, "Tying chain %s (%d) with %s (%d)\n",
> > + reg_names[c->regno], c->id,
> > + reg_names[head->regno], head->
RE: [RFC][PATCH] Preferred rename register in regrename pass
Hi, > On 11/09/2015 02:32 PM, Robert Suchanek wrote: > > The results below were generated for CSiBE benchmark and the numbers in > > columns express bytes in format 'net (gain/loss)' to show the difference > > with and without the patch when -frename-registers switch is used. > > I'm not entirely sure what the numbers represent. I can see how you'd > measure at a net size change (I assume a negative net is the intended > goal), but how did you arrive at gain/loss numbers? > > In any case, assuming negative is good, the results seem pretty decent. The gain/loss was calculated based on per function analysis. Each flavour e.g. MIPS n64 -Os was ran with/without the patch and compared to the base i.e. without the patch. The patched version of each function may show either positive (larger code size), negative or no difference to the code size. The gain/loss in a cell is the sum of all positive/negative numbers for a test. The negatives, as you said, are the good ones. > > > + gcc_assert > > + (terminated_this_insn->regno == REGNO (recog_data.operand[1])); > > Maybe break the line before the == so that you can start the arguments > on the same line as the assert. > > > + /* Nonzero if the chain is renamed. */ > > + unsigned int renamed:1; > > I'd write "has already been renamed" since that is maybe slightly less > ambiguous. > > Ok with those changes. > > > Bernd Will do the changes and apply. Regards, Robert
RE: [RFC][PATCH] Preferred rename register in regrename pass
Hi Christophe, > Hi, > > Since you committed this (r230087 if I'm correct), I can see that GCC > fails to build > ligfortran for target arm-none-linuxgnueabi --with-cpu=cortex-a9. ... > > Can you have a look? Sorry for the breakage. I see that my assertion is being triggered. I'll investigate this and check whether the assertion is correct or something else needs to be done. Robert
RE: [RFC][PATCH] Preferred rename register in regrename pass
Hi all,
> > Now that 'make check' has had enough time to run, I can see several
> > regressions in the configurations where GCC still builds.
> > For more details:
> > http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/230087/report-build-info.html
> >
>
> This also causes failures for AArch64 -mcpu=cortex-a57 targets. This
> testcase:
>
> void
> foo (unsigned char *out, const unsigned char *in, int a)
> {
> for (int i = 0; i < a; i++)
> {
> out[0] = in[2];
> out[1] = in[1];
> out[2] = in[0];
> in += 3;
> out += 3;
> }
> }
>
> Fails as so:
>
> foo.c: In function 'void foo(unsigned char*, const unsigned char*, int)':
> foo.c:12:1: internal compiler error: in scan_rtx_reg, at regrename.c:1074
>}
>^
>
> 0xbe00f8 scan_rtx_reg
> /gcc/regrename.c:1073
> 0xbe0ad5 scan_rtx
> /gcc/regrename.c:1401
> 0xbe1038 record_out_operands
> /gcc/regrename.c:1554
> 0xbe1f50 build_def_use
> /gcc/regrename.c:1802
> 0xbe1f50 regrename_analyze(bitmap_head*)
> /gcc/regrename.c:726
> 0xf7a0c7 func_fma_steering::execute_fma_steering()
> /gcc/config/aarch64/cortex-a57-fma-steering.c:1026
> 0xf7a9c1 pass_fma_steering::execute(function*)
> /gcc/config/aarch64/cortex-a57-fma-steering.c:1063
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See for instructions.
>
> When compiled with:
>
>-O3 -mcpu=cortex-a57 foo.c
>
> Thanks,
> James 0xbe1f50 build_def_use
> /gcc/regrename.c:1802
> 0xbe1f50 regrename_analyze(bitmap_head*)
> /gcc/regrename.c:726
> 0xf7a0c7 func_fma_steering::execute_fma_steering()
> /gcc/config/aarch64/cortex-a57-fma-steering.c:1026
> 0xf7a9c1 pass_fma_steering::execute(function*)
> /gcc/config/aarch64/cortex-a57-fma-steering.c:1063
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See for instructions.
>
> When compiled with:
>
>-O3 -mcpu=cortex-a57 foo.c
Thanks for the test case.
It appears that I managed to run only those tests that didn't expose
the assertion error and there is at least one more port i.e. powerpc64
showing similar ICEs when -funroll-loops and/or -fpeel-loops are used
that enables the regrename pass.
In both AArch64 and ARM cases I found the same insufficient checks
when chains are tied and it seems that this is the root cause behind
all failures.
With the attached patch I built arm-none-linux-gnueabi without failures,
checked a number of cases shown on Christophe's page, the above
test case, and it would appear that the problem is solved.
The reason behind the failures is that the terminated_this_insn had
a different number of consecutive registers (and mode) to the input
operand in a move currently being considered for tying. In the fix,
I allow tying only if there is matching number of NREGS.
Bernd, do you think that this check would be sufficient and safe?
I'm not sure what would be better: check the mode, nregs plus perhaps
consider tying only if nregs == 1.
Regards,
Robert
gcc/
* regname.c (scan_rtx_reg): Check the matching number of consecutive
registers when tying chains.
---
gcc/regrename.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gcc/regrename.c b/gcc/regrename.c
index d727dd9..0b8f032 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -1068,7 +1068,9 @@ scan_rtx_reg (rtx_insn *insn, rtx *loc, enum reg_class
cl, enum scan_actions act
&& GET_CODE (pat) == SET
&& GET_CODE (SET_DEST (pat)) == REG
&& GET_CODE (SET_SRC (pat)) == REG
- && terminated_this_insn)
+ && terminated_this_insn
+ && terminated_this_insn->nregs
+== REG_NREGS (recog_data.operand[1]))
{
gcc_assert (terminated_this_insn->regno
== REGNO (recog_data.operand[1]));
--
2.4.5
RE: [RFC][PATCH] Preferred rename register in regrename pass
Hi,
> > Bernd, do you think that this check would be sufficient and safe?
> > I'm not sure what would be better: check the mode, nregs plus perhaps
> > consider tying only if nregs == 1.
>
> Hmm, but shouldn't the regno still be the same? Or is this a case where
> we have a multi-word chain like ax/dx and then something like a "set bx,
> dx" involving only a part of it, but the entire chain dies?
The more I stare at this the more confusing it is. Yes, it appears to be a
multi-word
chain and when a subset dies then the whole chain dies.
Let's consider the following snippet:
...
(insn 1467 1465 1466 68 (set (reg:DI 4 r4 [626])
(mult:DI (zero_extend:DI (reg:SI 1 r1 [orig:698 bbase_yn ] [698]))
(zero_extend:DI (reg:SI 12 ip [orig:700 _302 ] [700]
/scratch2/check-other-ports/src/gcc/libgfortran/generated/matmul_i8.c:284 54
{*umulsidi3_v6}
(nil))
(insn 1466 1467 4288 68 (set (reg:SI 2 r2 [625])
(plus:SI (mult:SI (reg:SI 12 ip [orig:700 _302 ] [700])
(reg:SI 0 r0 [orig:699 bbase_yn+4 ] [699]))
(reg:SI 2 r2 [624])))
/scratch2/check-other-ports/src/gcc/libgfortran/generated/matmul_i8.c:284 43
{*mulsi3addsi_v6}
(expr_list:REG_DEAD (reg:SI 12 ip [orig:700 _302 ] [700])
(nil)))
(insn 4288 1466 1469 68 (set (reg:SI 12 ip [1933])
(reg:SI 5 r5 [+4 ]))
/scratch2/check-other-ports/src/gcc/libgfortran/generated/matmul_i8.c:284 174
{*arm_movsi_insn}
(expr_list:REG_DEAD (reg:SI 5 r5 [+4 ])
(nil)))
...
When the input operand in insn 4288 is terminated as dead then the
terminated_this_insn->regno points to register 4 but this_regno is 5.
terminated_this_insn->last->insn points to insn 1467.
I presume "[+4 ]" for register 5 in the dump indicates that this is a part of
the multi-word register.
When a new chain is created for the output operand with register 12
and tying is attempted then we get an assertion error.
> I guess this is ok to stop the failures for now, but you may want to
> move the check to the point where we set terminated_this_insn. Also, as
> I pointed out earlier, clearing terminated_this_insn should probably
> happen earlier.
>
> Bernd
Ah yes, I forgot to move this. I'll move it and commit the patch in the morning.
Regards,
Robert
RE: [RFC][PATCH] Preferred rename register in regrename pass
Hi,
> I guess this is ok to stop the failures for now, but you may want to
> move the check to the point where we set terminated_this_insn. Also, as
> I pointed out earlier, clearing terminated_this_insn should probably
> happen earlier.
Here is the updated patch that I'm about to commit once the bootstrap
finishes.
Regards,
Robert
gcc/
* regname.c (scan_rtx_reg): Check the matching number of consecutive
registers when tying chains.
(build_def_use): Move terminated_this_insn earlier in the function.
---
gcc/regrename.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/gcc/regrename.c b/gcc/regrename.c
index d727dd9..d41410a 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -1068,7 +1068,9 @@ scan_rtx_reg (rtx_insn *insn, rtx *loc, enum reg_class
cl, enum scan_actions act
&& GET_CODE (pat) == SET
&& GET_CODE (SET_DEST (pat)) == REG
&& GET_CODE (SET_SRC (pat)) == REG
- && terminated_this_insn)
+ && terminated_this_insn
+ && terminated_this_insn->nregs
+== REG_NREGS (recog_data.operand[1]))
{
gcc_assert (terminated_this_insn->regno
== REGNO (recog_data.operand[1]));
@@ -1593,6 +1595,7 @@ build_def_use (basic_block bb)
enum rtx_code set_code = SET;
enum rtx_code clobber_code = CLOBBER;
insn_rr_info *insn_info = NULL;
+ terminated_this_insn = NULL;
/* Process the insn, determining its effect on the def-use
chains and live hard registers. We perform the following
@@ -1749,8 +1752,6 @@ build_def_use (basic_block bb)
scan_rtx (insn, &XEXP (note, 0), ALL_REGS, mark_read,
OP_INOUT);
- terminated_this_insn = NULL;
-
/* Step 4: Close chains for registers that die here, unless
the register is mentioned in a REG_UNUSED note. In that
case we keep the chain open until step #7 below to ensure
--
2.4.
RE: [RFC][PATCH] Preferred rename register in regrename pass
Hi Christophe, > > > Hi, > I confirm that this fixes the build errors I was seeing. > Thanks. > Thanks for checking this. I'm still seeing a number of ICEs on the gcc-testresults mailing list across various ports but these are likely to be caused another patch. They are already reported as PR68293 and PR68296. Regards, Robert
[PATCH] MIPS: Add i6500 processor as an alias for i6400
Hi,
This patch adds i6500 CPU as an alias for i6400.
Regards,
Robert
gcc/ChangeLog:
2018-06-01 Matthew Fortune
* config/mips/mips-cpus.def: New MIPS_CPU for i6500.
* config/mips/mips-tables.opt: Regenerate.
* config/mips/mips.h (MIPS_ISA_LEVEL_SPEC): Mark i6500 as
mips64r6.
* doc/invoke.texi: Document -march=i6500.
---
gcc/config/mips/mips-cpus.def | 1 +
gcc/config/mips/mips-tables.opt | 3 +++
gcc/config/mips/mips.h | 2 +-
gcc/doc/invoke.texi | 2 +-
4 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/gcc/config/mips/mips-cpus.def b/gcc/config/mips/mips-cpus.def
index d0640e5..7314335 100644
--- a/gcc/config/mips/mips-cpus.def
+++ b/gcc/config/mips/mips-cpus.def
@@ -171,3 +171,4 @@ MIPS_CPU ("xlp", PROCESSOR_XLP, 65,
PTF_AVOID_BRANCHLIKELY_SPEED)
/* MIPS64 Release 6 processors. */
MIPS_CPU ("i6400", PROCESSOR_I6400, 69, 0)
+MIPS_CPU ("i6500", PROCESSOR_I6400, 69, 0)
diff --git a/gcc/config/mips/mips-tables.opt b/gcc/config/mips/mips-tables.opt
index daccefb..d8e50b2 100644
--- a/gcc/config/mips/mips-tables.opt
+++ b/gcc/config/mips/mips-tables.opt
@@ -696,3 +696,6 @@ Enum(mips_arch_opt_value) String(xlp) Value(101) Canonical
EnumValue
Enum(mips_arch_opt_value) String(i6400) Value(102) Canonical
+EnumValue
+Enum(mips_arch_opt_value) String(i6500) Value(103) Canonical
+
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index f290560..705434e 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -782,7 +782,7 @@ struct mips_cpu_info {
%{march=mips64r2|march=loongson3a|march=octeon|march=xlp: -mips64r2} \
%{march=mips64r3: -mips64r3} \
%{march=mips64r5: -mips64r5} \
- %{march=mips64r6|march=i6400: -mips64r6}}"
+ %{march=mips64r6|march=i6400|march=i6500: -mips64r6}}"
/* A spec that injects the default multilib ISA if no architecture is
specified. */
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 53ef14c..0d3a912 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -20220,7 +20220,7 @@ The processor names are:
@samp{34kc}, @samp{34kf2_1}, @samp{34kf1_1}, @samp{34kn},
@samp{74kc}, @samp{74kf2_1}, @samp{74kf1_1}, @samp{74kf3_2},
@samp{1004kc}, @samp{1004kf2_1}, @samp{1004kf1_1},
-@samp{i6400},
+@samp{i6400}, @samp{i6500},
@samp{interaptiv},
@samp{loongson2e}, @samp{loongson2f}, @samp{loongson3a},
@samp{m4k},
--
2.7.4
[PATCH] MIPS: Add support for -mcrc and -mginv options
Hi,
This patch adds -mcrc and -mginv options to pass through them
to the assembler.
Regards,
Robert
gcc/ChangeLog:
2018-06-01 Matthew Fortune
* config/mips/mips.h (ASM_SPEC): Pass through -mcrc, -mno-crc,
-mginv and -mno-ginv to the assembler.
* config/mips/mips.opt (-mcrc): New option.
(-mginv): Likewise.
* doc/invoke.text (-mcrc): Document.
(-mginv): Likewise.
---
gcc/config/mips/mips.h | 2 ++
gcc/config/mips/mips.opt | 8
gcc/doc/invoke.texi | 14 ++
3 files changed, 24 insertions(+)
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index f290560..13c30f0 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -1354,6 +1354,8 @@ struct mips_cpu_info {
%{meva} %{mno-eva} \
%{mvirt} %{mno-virt} \
%{mxpa} %{mno-xpa} \
+%{mcrc} %{mno-crc} \
+%{mginv} %{mno-ginv} \
%{mmsa} %{mno-msa} \
%{msmartmips} %{mno-smartmips} \
%{mmt} %{mno-mt} \
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index 545da54..5a9f255 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -412,6 +412,14 @@ mxpa
Target Report Var(TARGET_XPA)
Use eXtended Physical Address (XPA) instructions.
+mcrc
+Target Report Var(TARGET_CRC)
+Use Cyclic Redundancy Check (CRC) instructions.
+
+mginv
+Target Report Var(TARGET_GINV)
+Use Global INValidate (GINV) instructions.
+
mvr4130-align
Target Report Mask(VR4130_ALIGN)
Perform VR4130-specific alignment optimizations.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 53ef14c..bcb4c14 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -888,6 +888,8 @@ Objective-C and Objective-C++ Dialects}.
-meva -mno-eva @gol
-mvirt -mno-virt @gol
-mxpa -mno-xpa @gol
+-mcrc -mno-crc @gol
+-mginv -mno-ginv @gol
-mmicromips -mno-micromips @gol
-mmsa -mno-msa @gol
-mfpu=@var{fpu-type} @gol
@@ -20701,6 +20703,18 @@ Use (do not use) the MIPS Virtualization (VZ)
instructions.
@opindex mno-xpa
Use (do not use) the MIPS eXtended Physical Address (XPA) instructions.
+@item -mcrc
+@itemx -mno-crc
+@opindex mcrc
+@opindex mno-crc
+Use (do not use) the MIPS Cyclic Redundancy Check (CRC) instructions.
+
+@item -mginv
+@itemx -mno-ginv
+@opindex mginv
+@opindex mno-ginv
+Use (do not use) the MIPS Global INValidate (GINV) instructions.
+
@item -mlong64
@opindex mlong64
Force @code{long} types to be 64 bits wide. See @option{-mlong32} for
--
2.7.4
[PATCH] MIPS: Update I6400 scheduler
Hi, Update to i6400 scheduler. Regards, Robert gcc/ChangeLog: 2018-06-01 Prachi Godbole * config/mips/i6400.md (i6400_gpmuldiv): Remove cpu_unit. (i6400_gpmul): Add cpu_unit. (i6400_gpdiv): Likewise. (i6400_msa_add_d): Update reservations. (i6400_msa_int_add) Likewise. (i6400_msa_short_logic3) Likewise. (i6400_msa_short_logic2) Likewise. (i6400_msa_short_logic) Likewise. (i6400_msa_move) Likewise. (i6400_msa_cmp) Likewise. (i6400_msa_short_float2) Likewise. (i6400_msa_div_d) Likewise. (i6400_msa_long_logic1) Likewise. (i6400_msa_long_logic2) Likewise. (i6400_msa_mult) Likewise. (i6400_msa_long_float2) Likewise. (i6400_msa_long_float4) Likewise. (i6400_msa_long_float5) Likewise. (i6400_msa_long_float8) Likewise. (i6400_fpu_minmax): New define_insn_reservation. (i6400_fpu_fadd): Include frint type. (i6400_fpu_store): New define_insn_reservation. (i6400_fpu_load): Likewise. (i6400_fpu_move): Likewise. (i6400_fpu_fcmp): Likewise. (i6400_fpu_fmadd): Likewise. (i6400_int_mult): Include imul3nc type and update reservation. (i6400_int_div): Include idiv3 type and update reservation. (i6400_int_load): Update to check type not move_type. (i6400_int_store): Likewise. (i6400_int_prefetch): Set zero latency. --- gcc/config/mips/i6400.md | 86 ++-- 1 file changed, 61 insertions(+), 25 deletions(-) diff --git a/gcc/config/mips/i6400.md b/gcc/config/mips/i6400.md index 413e9e8..a985401 100644 --- a/gcc/config/mips/i6400.md +++ b/gcc/config/mips/i6400.md @@ -21,7 +21,7 @@ (define_automaton "i6400_int_pipe, i6400_mdu_pipe, i6400_fpu_short_pipe, i6400_fpu_long_pipe") -(define_cpu_unit "i6400_gpmuldiv" "i6400_mdu_pipe") +(define_cpu_unit "i6400_gpmul, i6400_gpdiv" "i6400_mdu_pipe") (define_cpu_unit "i6400_agen, i6400_alu1, i6400_lsu" "i6400_int_pipe") (define_cpu_unit "i6400_control, i6400_ctu, i6400_alu0" "i6400_int_pipe") @@ -50,49 +50,49 @@ (define_insn_reservation "i6400_msa_add_d" 1 (and (eq_attr "cpu" "i6400") (and (eq_attr "mode" "!V2DI") (eq_attr "alu_type" "simd_add"))) - "i6400_fpu_short, i6400_fpu_intadd") + "i6400_fpu_short+i6400_fpu_intadd*2") ;; add, hadd, sub, hsub, average, min, max, compare (define_insn_reservation "i6400_msa_int_add" 2 (and (eq_attr "cpu" "i6400") (eq_attr "type" "simd_int_arith")) - "i6400_fpu_short, i6400_fpu_intadd") + "i6400_fpu_short+i6400_fpu_intadd*2") ;; sat, pcnt (define_insn_reservation "i6400_msa_short_logic3" 3 (and (eq_attr "cpu" "i6400") (eq_attr "type" "simd_sat,simd_pcnt")) - "i6400_fpu_short, i6400_fpu_logic") + "i6400_fpu_short+i6400_fpu_logic*2") ;; shifts, nloc, nlzc, bneg, bclr, shf (define_insn_reservation "i6400_msa_short_logic2" 2 (and (eq_attr "cpu" "i6400") (eq_attr "type" "simd_shift,simd_shf,simd_bit")) - "i6400_fpu_short, i6400_fpu_logic") + "i6400_fpu_short+i6400_fpu_logic*2") ;; and, or, xor, ilv, pck, fill, splat (define_insn_reservation "i6400_msa_short_logic" 1 (and (eq_attr "cpu" "i6400") (eq_attr "type" "simd_permute,simd_logic,simd_splat,simd_fill")) - "i6400_fpu_short, i6400_fpu_logic") + "i6400_fpu_short+i6400_fpu_logic*2") ;; move.v, ldi (define_insn_reservation "i6400_msa_move" 1 (and (eq_attr "cpu" "i6400") (eq_attr "type" "simd_move")) - "i6400_fpu_short, i6400_fpu_logic") + "i6400_fpu_short+i6400_fpu_logic*2") ;; Float compare New: CMP.cond.fmt (define_insn_reservation "i6400_msa_cmp" 2 (and (eq_attr "cpu" "i6400") (eq_attr "type" "simd_fcmp")) - "i6400_fpu_short, i6400_fpu_cmp") + "i6400_fpu_short+i6400_fpu_cmp*2") ;; Float min, max, class (define_insn_reservation "i6400_msa_short_float2" 2 (and (eq_attr "cpu" "i6400") (eq_attr "type" "simd_fminmax,simd_fclass")) - "i6400_fpu_short, i6400_fpu_float") + "i6400_fpu_short+i6400_fpu_float*2") ;; div.d, mod.d (non-pipelined) (define_insn_reservation "i6400_msa_div_d" 36 @@ -158,43 +158,43 @@ (define_insn_reservation "i6400_fpu_msa_move" 1 (define_insn_reservation "i6400_msa_long_logic1" 1 (and (eq_attr "cpu" "i6400") (eq_attr "type" "simd_bitmov,simd_insert")) - "i6400_fpu_long, i6400_fpu_logic_l") + "i6400_fpu_long+i6400_fpu_logic_l*2") ;; binsl, binsr, vshf, sld (define_insn_reservation "i6400_msa_long_logic2" 2 (and (eq_attr "cpu" "i6400") (eq_attr "type" "simd_bitins,simd_sld")) - "i6400_fpu_long, i6400_fpu_logic_l") + "i6400_fpu_long+i6400_fpu_logic_l*2") ;; Vector mul, dotp, madd, msub (define_insn_reservation "i6400_msa_mult" 5 (and (eq_attr "cpu" "i6400") (eq_attr "type" "simd_mul")) - "i6400_fpu_long, i6400_fpu_mult") + "i6400_fpu_long+i6400_fpu_mult*2") ;; Float flog2 (defin
[PATCH] MIPS: Add support for P6600
Hi,
The below adds support for -march=p6600. It includes
a new scheduler plus performance tweaks.
gcc/ChangeLog:
2018-06-01 Matthew Fortune
Prachi Godbole
* config/mips/mips-cpus.def: Define P6600.
* config/mips/mips-tables.opt: Regenerate.
* config/mips/mips.c (mips_multipass_dfa_lookahead): Add P6600.
* config/mips/mips.h (TUNE_P6600): New define.
(MIPS_ISA_LEVEL_SPEC): Infer mips64r6 from p6600.
* doc/invoke.texi: Add p6600 to supported architectures.
---
gcc/config/mips/mips-cpus.def | 1 +
gcc/config/mips/mips-tables.opt | 3 +
gcc/config/mips/mips.c | 84 +-
gcc/config/mips/mips.h | 6 +-
gcc/config/mips/mips.md | 2 +
gcc/config/mips/p6600.md| 342
gcc/doc/invoke.texi | 2 +-
7 files changed, 434 insertions(+), 6 deletions(-)
create mode 100644 gcc/config/mips/p6600.md
diff --git a/gcc/config/mips/mips-cpus.def b/gcc/config/mips/mips-cpus.def
index d0640e5..c6d27b6 100644
--- a/gcc/config/mips/mips-cpus.def
+++ b/gcc/config/mips/mips-cpus.def
@@ -171,3 +171,4 @@ MIPS_CPU ("xlp", PROCESSOR_XLP, 65,
PTF_AVOID_BRANCHLIKELY_SPEED)
/* MIPS64 Release 6 processors. */
MIPS_CPU ("i6400", PROCESSOR_I6400, 69, 0)
+MIPS_CPU ("p6600", PROCESSOR_P6600, 69, 0)
diff --git a/gcc/config/mips/mips-tables.opt b/gcc/config/mips/mips-tables.opt
index daccefb..edf06af 100644
--- a/gcc/config/mips/mips-tables.opt
+++ b/gcc/config/mips/mips-tables.opt
@@ -696,3 +696,6 @@ Enum(mips_arch_opt_value) String(xlp) Value(101) Canonical
EnumValue
Enum(mips_arch_opt_value) String(i6400) Value(102) Canonical
+EnumValue
+Enum(mips_arch_opt_value) String(p6600) Value(105) Canonical
+
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index bfe64bb..9c77c13 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -198,6 +198,15 @@ enum mips_address_type {
ADDRESS_SYMBOLIC
};
+/* Classifies an unconditional branch of interest for the P6600. */
+
+enum mips_ucbranch_type {
+ /* May not even be a branch. */
+ UC_UNDEFINED,
+ UC_BALC,
+ UC_OTHER
+};
+
/* Macros to create an enumeration identifier for a function prototype. */
#define MIPS_FTYPE_NAME1(A, B) MIPS_##A##_FTYPE_##B
#define MIPS_FTYPE_NAME2(A, B, C) MIPS_##A##_FTYPE_##B##_##C
@@ -1127,6 +1136,19 @@ static const struct mips_rtx_cost_data
COSTS_N_INSNS (36), /* int_div_di */
2,/* branch_cost */
4 /* memory_latency */
+ },
+ { /* P6600 */
+COSTS_N_INSNS (4),/* fp_add */
+COSTS_N_INSNS (5),/* fp_mult_sf */
+COSTS_N_INSNS (5),/* fp_mult_df */
+COSTS_N_INSNS (17), /* fp_div_sf */
+COSTS_N_INSNS (17), /* fp_div_df */
+COSTS_N_INSNS (5),/* int_mult_si */
+COSTS_N_INSNS (5),/* int_mult_di */
+COSTS_N_INSNS (8),/* int_div_si */
+COSTS_N_INSNS (8),/* int_div_di */
+ 2,/* branch_cost */
+ 4 /* memory_latency */
}
};
@@ -14592,6 +14614,7 @@ mips_issue_rate (void)
case PROCESSOR_LOONGSON_2F:
case PROCESSOR_LOONGSON_3A:
case PROCESSOR_P5600:
+case PROCESSOR_P6600:
return 4;
case PROCESSOR_XLP:
@@ -14727,7 +14750,7 @@ mips_multipass_dfa_lookahead (void)
if (TUNE_OCTEON)
return 2;
- if (TUNE_P5600 || TUNE_I6400)
+ if (TUNE_P5600 || TUNE_P6600 || TUNE_I6400)
return 4;
return 0;
@@ -18647,6 +18670,28 @@ mips_orphaned_high_part_p (mips_offset_table *htab,
rtx_insn *insn)
return false;
}
+/* Subroutine of mips_avoid_hazard. We classify unconditional branches
+ of interest for the P6600 for performance reasons. We're interested
+ in differentiating BALC from JIC, JIALC and BC. */
+
+static enum mips_ucbranch_type
+mips_classify_branch_p6600 (rtx_insn *insn)
+{
+ if (!(insn
+ && USEFUL_INSN_P (insn)
+ && GET_CODE (PATTERN (insn)) != SEQUENCE))
+return UC_UNDEFINED;
+
+ if (get_attr_jal (insn) == JAL_INDIRECT /* JIC and JIALC. */
+ || get_attr_type (insn) == TYPE_JUMP) /* BC as it is a loose jump. */
+return UC_OTHER;
+
+ if (CALL_P (insn) && get_attr_jal (insn) == JAL_DIRECT)
+return UC_BALC;
+
+ return UC_UNDEFINED;
+}
+
/* Subroutine of mips_reorg_process_insns. If there is a hazard between
INSN and a previous instruction, avoid it by inserting nops after
instruction AFTER.
@@ -18699,14 +18744,36 @@ mips_avoid_hazard (rtx_insn *after, rtx_insn *insn,
int *hilo_delay,
&& GET_CODE (pattern) != ASM_INPUT
&& asm_noperands (pattern) < 0)
nops = 1;
+ /* The P6600's branch predictor does not handle certain static
+ sequences of back-to-back jumps well. Inserting a no-op only
+ costs space as the dispatch unit will disregard the nop.
+ Here we handle
RE: [PATCH] MIPS: Update I6400 scheduler
Hi Matthew, > There does seem to be a temporal issue in submission for this as the > i6400_fpu_minmax reservation refers to fminmax and fclass types that > do not exist in trunk. Can you drop that reservation please? > > Otherwise, OK to commit. Reservation dropped. Committed as r261489. Regards, Robert
RE: [PATCH] MIPS: Add i6500 processor as an alias for i6400
Hi, > Looks good, OK to commit. Committed as r261490. Regards, Robert
RE: [PATCH] MIPS: Update I6400 scheduler
Hi, > > There does seem to be a temporal issue in submission for this as the > > i6400_fpu_minmax reservation refers to fminmax and fclass types that > > do not exist in trunk. Can you drop that reservation please? > > > > Otherwise, OK to commit. > > Reservation dropped. > > Committed as r261489. There was also a reference to frint type that does not exist in the trunk. Committed as obvious (r261494). Regards, Robert gcc/ * config/mips/i6400.md (i6400_fpu_fadd): Remove frint. diff --git a/gcc/config/mips/i6400.md b/gcc/config/mips/i6400.m index 8058a8dd807..97f6c0b44e7 100644 --- a/gcc/config/mips/i6400.md +++ b/gcc/config/mips/i6400.md @@ -222,7 +222,7 @@ (define_insn_reservation "i6400_fpu_fabs" 1 ;; fadd, fsub, fcvt (define_insn_reservation "i6400_fpu_fadd" 4 (and (eq_attr "cpu" "i6400") - (eq_attr "type" "fadd, fcvt, frint")) + (eq_attr "type" "fadd,fcvt")) "i6400_fpu_long, i6400_fpu_apu") ;; fmul
RE: [PATCH] MIPS: Add support for P6600
Hi Matthew,
As already discussed, the link to the P6600 doesn't
appear to be referenced on mips.com but reachable
when searching for 'p6600':
https://www.mips.com/downloads/p6600-multiprocessing-programmers-guide/
I'm resubmitting the whole patch again with updated
ChangeLog.
> >
> > +/* Classifies an unconditional branch of interest for the P6600. */
> > +
> > +enum mips_ucbranch_type {
>
> Newline for brace.
Done.
> > +/* Subroutine of mips_avoid_hazard. We classify unconditional
> > branches
> > + of interest for the P6600 for performance reasons. We're
> > interested
> > + in differentiating BALC from JIC, JIALC and BC. */
> > +
> > +static enum mips_ucbranch_type
> > +mips_classify_branch_p6600 (rtx_insn *insn) {
> > + if (!(insn
> > + && USEFUL_INSN_P (insn)
> > + && GET_CODE (PATTERN (insn)) != SEQUENCE))
> > +return UC_UNDEFINED;
>
> Let's switch this around to the following with a comment to say ignore
> sequences because they represent a filled delay slot branch (which
> presumably is not affected by the uArch issue).
>
> if (insn
> || !USEFUL_INSN_P (insn)
> || GET_CODE (PATTERN (insn)) == SEQUENCE))
> return UC_UNDEFINED;
Switched around and added a comment.
>
> > +
> > + if (get_attr_jal (insn) == JAL_INDIRECT /* JIC and JIALC. */
> > + || get_attr_type (insn) == TYPE_JUMP) /* BC as it is a loose
> > jump. */
> > +return UC_OTHER;
>
> I don't recall what a loose jump was supposed to refer to, presumably
> 'direct jump'.
Left it just as 'BC'.
>
> > /* Subroutine of mips_reorg_process_insns. If there is a hazard
> > between
> > INSN and a previous instruction, avoid it by inserting nops after
> > instruction AFTER.
> > @@ -18699,14 +18744,36 @@ mips_avoid_hazard (rtx_insn *after, rtx_insn
> > *insn, int *hilo_delay,
> >&& GET_CODE (pattern) != ASM_INPUT
> >&& asm_noperands (pattern) < 0)
> > nops = 1;
> > + /* The P6600's branch predictor does not handle certain static
> > + sequences of back-to-back jumps well. Inserting a no-op only
> > + costs space as the dispatch unit will disregard the nop.
> > + Here we handle the cases of back to back unconditional branches
> > + that are inefficent. */
> > + else if (TUNE_P6600 && TARGET_CB_MAYBE && !optimize_size
> > + && ((mips_classify_branch_p6600 (after) == UC_BALC
> > + && mips_classify_branch_p6600 (insn) == UC_OTHER)
> > + || (mips_classify_branch_p6600 (insn) == UC_BALC
> > + && (mips_classify_branch_p6600 (after) == UC_OTHER
>
> Unnecessary braces on the last clause here.
Removed the braces and change the comment.
>
> > +nops = 1;
>
> This appears to show that a BALC with a compact branch (other than BALC)
> is the problem case either before or after the BALC. This is what we need
> to see in a public document. Or at least (re)confirmed as the issue.
>
> >else
> > nops = 0;
> >
> >/* Insert the nops between this instruction and the previous one.
> > Each new nop takes us further from the last hilo hazard. */
> >*hilo_delay += nops;
> > +
> > + /* If we're tuning for the P6600, we come across an annoying GCC
> > + assumption that debug information always follows a call. Move
> > + past any debug information in that case. */
> > + rtx_insn *real_after = after;
> > + if (real_after && nops && CALL_P (real_after))
> > +while (TUNE_P6600 && real_after
> > + && (NOTE_P (NEXT_INSN (real_after))
> > + || BARRIER_P (NEXT_INSN (real_after
> > + real_after = NEXT_INSN (real_after);
> > +
>
> This looks OK but it seems unnecessary to limit to the P6600. It is just
> that we have not had hazards on branches before now.
Removed the restriction.
>
> >while (nops-- > 0)
> > -emit_insn_after (gen_hazard_nop (), after);
> > +emit_insn_after (gen_hazard_nop (), real_after);
> >
> >/* Set up the state for the next instruction. */
> >*hilo_delay += ninsns;
> > @@ -18716,6 +18783,14 @@ mips_avoid_hazard (rtx_insn *after, rtx_insn
> > *insn, int *hilo_delay,
> > switch (get_attr_hazard (insn))
> >{
> >case HAZARD_NONE:
> > + /* For the P6600, flag some unconditional branches as having a
> > + pseudo-forbidden slot. This will cause additional nop
> > insertion
> > + or SEQUENCE breaking as required. */
> > + if (TUNE_P6600
> > + && !optimize_size
> > + && TARGET_CB_MAYBE
> > + && mips_classify_branch_p6600 (insn) == UC_OTHER)
> > + *fs_delay = true;
>
> This appears to be saying that all unconditional branches (except
> BALC) will introduce a performance penalty if followed by any other non-
> delay/non-forbidden slot instruction.
Added a comment that this is for performance reasons.
>
> > break;
> >
> >case HAZARD_FORBIDDEN_SLOT:
> > @@ -18957,7 +19032,10 @@ mips_reorg_process_insns (void)
> > sequence and replace it with
RE: [PATCH] MIPS: Add support for P6600
Hi Matthew,
> With one more change to add another comment as below, this is OK to
> commit.
>
> > @@ -18957,7 +19039,10 @@ mips_reorg_process_insns (void)
> > sequence and replace it with the delay slot instruction
> > then the jump to clear the forbidden slot hazard. */
>
> This bit does need the comment extending. Add this:
>
> For the P6600, this optimisation solves the performance penalty associated
> with BALC followed by a delay slot branch. We do not set fs_delay as we
> do not want the full logic of a forbidden slot; the penalty exists only
> against branches not the full class of forbidden slot instructions.
>
> >
> > - if (fs_delay)
> > + if (fs_delay || (TUNE_P6600
> > + && TARGET_CB_MAYBE
> > + && mips_classify_branch_p6600 (insn)
> > + == UC_BALC))
> > {
> > /* Search onwards from the current position looking for
> > a SEQUENCE. We are looking for pipeline hazards
> here
Added and committed as r261570.
Regards,
Robert
RE: [PATCH] MIPS: Add support for -mcrc and -mginv options
Hi, > Since CRC and GINV ASEs have now been committed to binutils, please go > ahead with this change. This is now committed as r261635. Robert
[RFC, PATCH][LRA, MIPS] ICE: in decompose_normal_address, at rtlanal.c:5817
Hi Vladimir,
The following testcase fails when compiled with -O2 -mips32r2:
long long a[];
long long b, c, d, k, m, n, o, p, q, r, s, t, u, v, w;
int e, f, g, h, i, j, l, x;
fn1() {
for (; x; x++)
if (x & 1)
s = h | g;
else
s = f | e;
l = ~0;
m = 1 | k;
n = i;
o = j;
p = f | e;
q = h | g;
w = d | c | a[1];
t = c;
v = b | c;
u = v;
r = b | a[4];
}
It is reproducible on mips-linux-gnu using SVN revision 212763. After a patch
to p5600.md the bug is not triggered but it may reappear in the future.
The decompose_normal_address function throws an assertion because it cannot
decompose the following RTL:
(mem/c:SI (lo_sum:SI (high:SI (symbol_ref:SI ("w")))
(const:SI (plus:SI (symbol_ref:SI ("w"))
(const_int 4 [0x4]))
It appears that it all starts in the following instruction and how LRA deals
with REG_EQUIV notes:
(insn 107 119 123 10 (set (reg:SI 283)
(ior:SI (reg:SI 284 [ a+12 ])
(reg:SI 309 [ D.1467+4 ]))) init.i:16 163 {*iorsi3}
(expr_list:REG_DEAD (reg:SI 309 [ D.1467+4 ])
(expr_list:REG_DEAD (reg:SI 284 [ a+12 ])
(expr_list:REG_EQUIV (mem/c:SI (lo_sum:SI (reg/f:SI 274)
(const:SI (plus:SI (symbol_ref:SI ("w"))
(const_int 4 [0x4])
(nil)
There are two conditions necessary to trigger the ICE.
Firstly, the pseudo 274 is spilled to memory that is marked by IRA, thus,
during LRA pass the pseudo 274 is replaced with 'high' when equivalences
are updated for pseudos. Secondly, pseudo 283 also gets spilled and LRA starts
using equivalences but LO_SUM and HIGH are already combined leading to
an assertion error.
Accepting HIGH as the base does seem to solve the problem. HIGH is also
reloaded after the decomposition splitting HIGH/LO_SUM into a pair again.
However, is this an acceptable solution?
Regards,
Robert
gcc/
* rtlanal.c (get_base_term): Accept HIGH as the base term.
diff --git gcc/rtlanal.c gcc/rtlanal.c
index 82cfc1bf..2bea2ca 100644
--- gcc/rtlanal.c
+++ gcc/rtlanal.c
@@ -5624,6 +5624,7 @@ get_base_term (rtx *inner)
inner = strip_address_mutations (&XEXP (*inner, 0));
if (REG_P (*inner)
|| MEM_P (*inner)
+ || GET_CODE (*inner) == HIGH
|| GET_CODE (*inner) == SUBREG)
return inner;
return 0;
[PATCH] LRA: check_rtl modifies RTL instruction stream
Hi, This patch follows up the problem outlined here: http://gcc.gnu.org/ml/gcc/2013-11/msg00152.html The patch attached prevents adding clobbers when LRA is running in insn_invalid_p function and lra_in_progress variable is being set just before check_rtl call. This should stop modification of insns stream in cases where there is a insn match after adding clobbers. The patch was bootstrapped and regtested on x86_64-unknown-linux-gnu (revision 204787). Regards, Robert 2013-11-13 Robert Suchanek * lra.c (lra): Set lra_in_progress before check_rtl call. * recog.c (insn_invalid_p): Add !lra_in_progress to prevent adding clobber regs when LRA is running diff --git a/gcc/lra.c b/gcc/lra.c index 1aea599..83d45b6 100644 --- a/gcc/lra.c +++ b/gcc/lra.c @@ -2238,6 +2238,10 @@ lra (FILE *f) init_insn_recog_data (); + /* We can not set up reload_in_progress because it prevents new + pseudo creation. */ + lra_in_progress = 1; + #ifdef ENABLE_CHECKING check_rtl (false); #endif @@ -2248,10 +2252,6 @@ lra (FILE *f) setup_reg_spill_flag (); - /* We can not set up reload_in_progress because it prevents new - pseudo creation. */ - lra_in_progress = 1; - /* Function remove_scratches can creates new pseudos for clobbers -- so set up lra_constraint_new_regno_start before its call to permit changing reg classes for pseudos created by this diff --git a/gcc/recog.c b/gcc/recog.c index c8594bb..5c0ec16 100644 --- a/gcc/recog.c +++ b/gcc/recog.c @@ -314,7 +314,9 @@ insn_invalid_p (rtx insn, bool in_group) clobbers. */ int icode = recog (pat, insn, (GET_CODE (pat) == SET - && ! reload_completed && ! reload_in_progress) + && ! reload_completed + && ! reload_in_progress + && ! lra_in_progress) ? &num_clobbers : 0); int is_asm = icode < 0 && asm_noperands (PATTERN (insn)) >= 0;
[PING][PATCH] LRA: check_rtl modifies RTL instruction stream
Hi, I'm pinging for the review of a patch. Original post: http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01608.html The attached patch is slightly modified as the indentation was incorrect and realised that after sending the email last time. If the patch is OK, could someone commit the patch? I do not have permission to do so. Regards, Robert 2013-11-13 Robert Suchanek * lra.c (lra): Set lra_in_progress before check_rtl call. * recog.c (insn_invalid_p): Add !lra_in_progress to prevent adding clobber regs when LRA is running diff --git a/gcc/lra.c b/gcc/lra.c index 1aea599..83d45b6 100644 --- a/gcc/lra.c +++ b/gcc/lra.c @@ -2238,6 +2238,10 @@ lra (FILE *f) init_insn_recog_data (); + /* We can not set up reload_in_progress because it prevents new + pseudo creation. */ + lra_in_progress = 1; + #ifdef ENABLE_CHECKING check_rtl (false); #endif @@ -2248,10 +2252,6 @@ lra (FILE *f) setup_reg_spill_flag (); - /* We can not set up reload_in_progress because it prevents new - pseudo creation. */ - lra_in_progress = 1; - /* Function remove_scratches can creates new pseudos for clobbers -- so set up lra_constraint_new_regno_start before its call to permit changing reg classes for pseudos created by this diff --git a/gcc/recog.c b/gcc/recog.c index c8594bb..5c0ec16 100644 --- a/gcc/recog.c +++ b/gcc/recog.c @@ -314,7 +314,9 @@ insn_invalid_p (rtx insn, bool in_group) clobbers. */ int icode = recog (pat, insn, (GET_CODE (pat) == SET - && ! reload_completed && ! reload_in_progress) + && ! reload_completed + && ! reload_in_progress + && ! lra_in_progress) ? &num_clobbers : 0); int is_asm = icode < 0 && asm_noperands (PATTERN (insn)) >= 0;
RE: [PING][PATCH] LRA: check_rtl modifies RTL instruction stream
>Thanks. Vlad may not be available right now, and even if he is, he's >probably typing one-handed. > >So I took care of installing this for you. > >Thanks, >Jeff Thanks! Regards, Robert
RE: RFA: Make LRA temporarily eliminate addresses before testing constraints
Hi Richard,
>> Robert: you also had an LRA change, but is it still needed after this one?
>> If so, could you repost it and explain the case it handles?
For just turning the LRA for the MIPS backend is not needed but we have issues
with the code size for MIPS16. LRA inserted a lot of reloads and the code size
increased on average by about 10% IIRC. To fix this, a number of patterns
have to accept the stack pointer and a new class, M16_SP_REGS with
M16_REGS + $sp was added.
However, this triggered a reloading problem as the stack pointer was rejected
by the back end and LRA tried to insert base+disp with the displacement not
always present. It only affects $sp not directly accessible as in MIPS16 case.
Regards,
Robert
gcc/
* lra-constraints.c (base_to_reg): New function.
(process_address): Use new function.
diff --git gcc/lra-constraints.c gcc/lra-constraints.c
index 08716fe..d5ed37f 100644
--- gcc/lra-constraints.c
+++ gcc/lra-constraints.c
@@ -2686,6 +2686,39 @@ process_alt_operands (int only_alternative)
return ok_p;
}
+/* Make reload base reg from address AD. */
+static rtx
+base_to_reg (struct address_info *ad)
+{
+ enum reg_class cl;
+ int code = -1;
+ rtx new_inner = NULL_RTX;
+ rtx new_reg = NULL_RTX;
+ rtx insn;
+ rtx last_insn = get_last_insn();
+
+ lra_assert (ad->base == ad->base_term && ad->disp == ad->disp_term);
+ cl = base_reg_class (ad->mode, ad->as, ad->base_outer_code,
+ get_index_code (ad));
+ new_reg = lra_create_new_reg (GET_MODE (*ad->base_term), NULL_RTX,
+cl, "base");
+ new_inner = simplify_gen_binary (PLUS, GET_MODE (new_reg), new_reg,
+ ad->disp_term == NULL
+ ? gen_int_mode (0, ad->mode)
+ : *ad->disp_term);
+ if (!valid_address_p (ad->mode, new_inner, ad->as))
+return NULL_RTX;
+ insn = emit_insn (gen_rtx_SET (ad->mode, new_reg, *ad->base_term));
+ code = recog_memoized (insn);
+ if (code < 0)
+{
+ delete_insns_since (last_insn);
+ return NULL_RTX;
+}
+
+ return new_inner;
+}
+
/* Make reload base reg + disp from address AD. Return the new pseudo. */
static rtx
base_plus_disp_to_reg (struct address_info *ad)
@@ -2908,6 +2941,8 @@ process_address_1 (int nop, rtx *before, rtx *after)
3) the address is a frame address with an invalid offset.
+ 4) the address is a frame address with an invalid base.
+
All these cases involve a non-autoinc address, so there is no
point revalidating other types. */
if (ad.autoinc_p || valid_address_p (&ad))
@@ -2989,14 +3024,19 @@ process_address_1 (int nop, rtx *before, rtx *after)
int regno;
enum reg_class cl;
rtx set, insns, last_insn;
+ /* Try to reload base into register only if the base is invalid
+ for the address but with valid offset, case (4) above. */
+ start_sequence ();
+ new_reg = base_to_reg (&ad);
+
/* base + disp => new base, cases (1) and (3) above. */
/* Another option would be to reload the displacement into an
index register. However, postreload has code to optimize
address reloads that have the same base and different
displacements, so reloading into an index register would
not necessarily be a win. */
- start_sequence ();
- new_reg = base_plus_disp_to_reg (&ad);
+ if (new_reg == NULL_RTX)
+new_reg = base_plus_disp_to_reg (&ad);
insns = get_insns ();
last_insn = get_last_insn ();
/* If we generated at least two insns, try last insn source as
RE: RFA: Make LRA temporarily eliminate addresses before testing constraints
Pinging for approval.
This part of the patch will be needed for MIPS16. The second part to enable
LRA in MIPS has been already approved.
> Hi Richard,
>
> >> Robert: you also had an LRA change, but is it still needed after this one?
> >> If so, could you repost it and explain the case it handles?
>
> For just turning the LRA for the MIPS backend is not needed but we have issues
> with the code size for MIPS16. LRA inserted a lot of reloads and the code size
> increased on average by about 10% IIRC. To fix this, a number of patterns
> have to accept the stack pointer and a new class, M16_SP_REGS with
> M16_REGS + $sp was added.
>
> However, this triggered a reloading problem as the stack pointer was rejected
> by the back end and LRA tried to insert base+disp with the displacement not
> always present. It only affects $sp not directly accessible as in MIPS16 case.
>
> Regards,
> Robert
>
> gcc/
> * lra-constraints.c (base_to_reg): New function.
> (process_address): Use new function.
>
> diff --git gcc/lra-constraints.c gcc/lra-constraints.c
> index 08716fe..d5ed37f 100644
> --- gcc/lra-constraints.c
> +++ gcc/lra-constraints.c
> @@ -2686,6 +2686,39 @@ process_alt_operands (int only_alternative)
>return ok_p;
> }
>
> +/* Make reload base reg from address AD. */
> +static rtx
> +base_to_reg (struct address_info *ad)
> +{
> + enum reg_class cl;
> + int code = -1;
> + rtx new_inner = NULL_RTX;
> + rtx new_reg = NULL_RTX;
> + rtx insn;
> + rtx last_insn = get_last_insn();
> +
> + lra_assert (ad->base == ad->base_term && ad->disp == ad->disp_term);
> + cl = base_reg_class (ad->mode, ad->as, ad->base_outer_code,
> + get_index_code (ad));
> + new_reg = lra_create_new_reg (GET_MODE (*ad->base_term), NULL_RTX,
> +cl, "base");
> + new_inner = simplify_gen_binary (PLUS, GET_MODE (new_reg), new_reg,
> + ad->disp_term == NULL
> + ? gen_int_mode (0, ad->mode)
> + : *ad->disp_term);
> + if (!valid_address_p (ad->mode, new_inner, ad->as))
> +return NULL_RTX;
> + insn = emit_insn (gen_rtx_SET (ad->mode, new_reg, *ad->base_term));
> + code = recog_memoized (insn);
> + if (code < 0)
> +{
> + delete_insns_since (last_insn);
> + return NULL_RTX;
> +}
> +
> + return new_inner;
> +}
> +
> /* Make reload base reg + disp from address AD. Return the new pseudo. */
> static rtx
> base_plus_disp_to_reg (struct address_info *ad)
> @@ -2908,6 +2941,8 @@ process_address_1 (int nop, rtx *before, rtx *after)
>
> 3) the address is a frame address with an invalid offset.
>
> + 4) the address is a frame address with an invalid base.
> +
> All these cases involve a non-autoinc address, so there is no
> point revalidating other types. */
>if (ad.autoinc_p || valid_address_p (&ad))
> @@ -2989,14 +3024,19 @@ process_address_1 (int nop, rtx *before, rtx *after)
>int regno;
>enum reg_class cl;
>rtx set, insns, last_insn;
> + /* Try to reload base into register only if the base is invalid
> + for the address but with valid offset, case (4) above. */
> + start_sequence ();
> + new_reg = base_to_reg (&ad);
> +
>/* base + disp => new base, cases (1) and (3) above. */
>/* Another option would be to reload the displacement into an
>index register. However, postreload has code to optimize
>address reloads that have the same base and different
>displacements, so reloading into an index register would
>not necessarily be a win. */
> - start_sequence ();
> - new_reg = base_plus_disp_to_reg (&ad);
> + if (new_reg == NULL_RTX)
> +new_reg = base_plus_disp_to_reg (&ad);
>insns = get_insns ();
>last_insn = get_last_insn ();
>/* If we generated at least two insns, try last insn source as
[PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
Hi,
This patch fixes an internal compiler error when micromips/nomicromips
attributes are used.
The problem here was that the cached boolean attributes for the current
target did not agree with the uncached attributes throwing an assertion error.
It appears that saving and restoring the state for micromips was missing,
just like there is for mips16. OK to apply?
Regards,
Robert
gcc/
* config/mips/mips.c (micromips_globals): New variable.
(mips_set_compression_mode): Save and reinitialize target-dependent
state for microMIPS.
gcc/testsuite
* gcc.target/mips/umips-attr.c: New test.
---
gcc/config/mips/mips.c | 10 ++
gcc/testsuite/gcc.target/mips/umips-attr.c | 13 +
2 files changed, 23 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/mips/umips-attr.c
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index bf69850..959a672 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -676,6 +676,9 @@ const char *mips_hi_relocs[NUM_SYMBOL_TYPES];
/* Target state for MIPS16. */
struct target_globals *mips16_globals;
+/* Target state for MICROMIPS. */
+struct target_globals *micromips_globals;
+
/* Cached value of can_issue_more. This is cached in mips_variable_issue hook
and returned from mips_sched_reorder2. */
static int cached_can_issue_more;
@@ -17182,6 +17185,13 @@ mips_set_compression_mode (unsigned int
compression_mode)
else
restore_target_globals (mips16_globals);
}
+ else if (compression_mode & MASK_MICROMIPS)
+{
+ if (!micromips_globals)
+ micromips_globals = save_target_globals_default_opts ();
+ else
+ restore_target_globals (micromips_globals);
+}
else
restore_target_globals (&default_target_globals);
diff --git a/gcc/testsuite/gcc.target/mips/umips-attr.c
b/gcc/testsuite/gcc.target/mips/umips-attr.c
new file mode 100644
index 000..f8c4517
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/umips-attr.c
@@ -0,0 +1,13 @@
+/* { dg-options "(-mmicromips)" } */
+
+int MICROMIPS
+foo (int a)
+{
+ return a;
+}
+
+int NOMICROMIPS
+foo2 (int a)
+{
+ return a;
+}
--
2.2.2
RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
Hi Matthew, > > This patch fixes an internal compiler error when micromips/nomicromips > > attributes are used. > > > > The problem here was that the cached boolean attributes for the current > > target did not agree with the uncached attributes throwing an assertion > > error. > > > > It appears that saving and restoring the state for micromips was > > missing, just like there is for mips16. OK to apply? > > Just to check, the reason we don't see this in the current testsuite is that > there is no test with both MIPS and microMIPS functions? Correct. The testsuite either tests microMIPS or MIPS but not both in the same TU. > > gcc/ > > * config/mips/mips.c (micromips_globals): New variable. > > (mips_set_compression_mode): Save and reinitialize target-dependent > > state for microMIPS. > > > > gcc/testsuite > > * gcc.target/mips/umips-attr.c: New test. > > OK. Committed as r223294. Regards, Robert
RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
Hi, The original patch had a missing declaration of micromips_globals in mips.h that appears to be the cause of segmentation faults when building mips-mti-linux-gnu. I didn't get any failures just before the submission neither on mips-img-linux-gnu nor mips64el-linux-gnu and the test case is too trivial to trigger the ICE. Below is the missing line. With this change mips-mti-linux-gnu builds fine. The trunk is unstable and needed another patch from PR66181 to build Glibc. Ok to commit? > We could add -mflip-micromips complementing -mflip-mips16 and use that > for testing too. Chances are it'd reveal further issues. Looking at how > -mflip-mips16 has been implemented it does not appear to me adding > -mflip-micromips would be a lot of effort. I'm in favour of adding such a switch since the testsuite doesn't cover a mixture of MIPS and microMIPS code. Regards, Robert gcc/ * config/mips/mips.h (micromips_globals): Declare. --- gcc/config/mips/mips.h | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index 0ea4e6d..85c8a97 100644 --- a/gcc/config/mips/mips.h +++ b/gcc/config/mips/mips.h @@ -3108,6 +3108,7 @@ extern const struct mips_cpu_info *mips_arch_info; extern const struct mips_cpu_info *mips_tune_info; extern unsigned int mips_base_compression_flags; extern GTY(()) struct target_globals *mips16_globals; +extern GTY(()) struct target_globals *micromips_globals; #endif /* Enable querying of DFA units. */ -- 2.2.2
RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
> > gcc/ > > * config/mips/mips.h (micromips_globals): Declare. > > OK, thanks. > > Matthew Committed as r223438. Robert
[Patch MIPS] Enable TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook
Hi,
The patch enables the hook for MIPS as a result of the discussion:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65862
Tested on mips-mti-linux-gnu and mips-img-linux-gnu. Ok to apply?
Regards,
Robert
gcc/ChangeLog:
* config/mips/mips.c (mips_ira_change_pseudo_allocno_class): New
function.
(TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS): Define macro.
gcc/testsuite/ChangeLog:
* gcc.target/mips/pr65862-1.c: New test.
* gcc.target/mips/pr65862-2.c: Likewise.
---
gcc/config/mips/mips.c| 13 +
gcc/testsuite/gcc.target/mips/pr65862-1.c | 16
gcc/testsuite/gcc.target/mips/pr65862-2.c | 31 +++
3 files changed, 60 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/mips/pr65862-1.c
create mode 100644 gcc/testsuite/gcc.target/mips/pr65862-2.c
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index c3755f5..3c8ac30 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19415,6 +19415,17 @@ mips_lra_p (void)
{
return mips_lra_flag;
}
+
+/* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS. */
+
+static reg_class_t
+mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
+{
+ if (FLOAT_MODE_P (PSEUDO_REGNO_MODE (regno)) || allocno_class != ALL_REGS)
+return allocno_class;
+ return GR_REGS;
+}
+
/* Initialize the GCC target structure. */
#undef TARGET_ASM_ALIGNED_HI_OP
@@ -19671,6 +19682,8 @@ mips_lra_p (void)
#define TARGET_SPILL_CLASS mips_spill_class
#undef TARGET_LRA_P
#define TARGET_LRA_P mips_lra_p
+#undef TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
+#define TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
mips_ira_change_pseudo_allocno_class
struct gcc_target targetm = TARGET_INITIALIZER;
diff --git a/gcc/testsuite/gcc.target/mips/pr65862-1.c
b/gcc/testsuite/gcc.target/mips/pr65862-1.c
new file mode 100644
index 000..0c00092
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/pr65862-1.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
+/* { dg-final { scan-assembler-not "\\\$f\[0-9\]+" } } */
+int a, c;
+int *b, *d;
+void
+fn1(int p1, int *p2(void *, void *), void *p3(void *, void *, int)) {
+ int n = c;
+ for (;;) {
+a = 1;
+for (; a < n;) {
+ *d = p1 && p2(0, (int *) ((long)p1 + 1));
+ p3(0, b + p1, 0);
+}
+ }
+}
diff --git a/gcc/testsuite/gcc.target/mips/pr65862-2.c
b/gcc/testsuite/gcc.target/mips/pr65862-2.c
new file mode 100644
index 000..c6a2641
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/pr65862-2.c
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
+/* { dg-final { scan-assembler-not "\\\$f\[0-9\]+" } } */
+int a, b, d, e, j, k, n, o;
+unsigned c, h, i, l, m, p;
+int *f;
+int *g;
+int fn1(int p1) { return p1 - a; }
+
+int fn2() {
+ b = b + 1 - a;
+ e = 1 + o + 1518500249;
+ d = d + n;
+ c = (int)c + g[0];
+ b = b + m + 1;
+ d = d + p + 1518500249;
+ d = d + k - 1;
+ c = fn1(c + j + 1518500249);
+ e = fn1(e + i + 1);
+ d = d + h + 1859775393 - a;
+ c = fn1(c + (d ^ 1 ^ b) + g[1] + 1);
+ b = fn1(b + m + 3);
+ d = fn1(d + l + 1);
+ b = b + (c ^ 1) + p + 1;
+ e = fn1(e + (b ^ c ^ d) + n + 1);
+ d = o;
+ b = 0;
+ e = e + k + 1859775393;
+ f[0] = e;
+ return a;
+}
--
2.2.2
RE: [Patch MIPS] Enable TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook
Hi Matthew,
> > +
> > +/* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS. */
> > +
> > +static reg_class_t
> > +mips_ira_change_pseudo_allocno_class (int regno, reg_class_t
> > +allocno_class) {
> > + if (FLOAT_MODE_P (PSEUDO_REGNO_MODE (regno)) || allocno_class !=
> > ALL_REGS)
> > +return allocno_class;
> > + return GR_REGS;
> > +}
> > +
>
> I'm concerned that this may not be the right condition but either way,
> I think it is better to switch this around to have the special case
> as the conditional. I found it difficult to understand what it is
> doing even when I know the intent :-) A comment about the purpose seems
> appropriate too here as it won't be obvious to someone new.
I tried to write a sensible comment and found the original change hard
to describe. I changed the condition to the special case and did some
experiments. The patch below is now more concise, better fits the purpose and
it
seems to have marginally better allocation too.
> Aren't there some fixed point modes that should go in FPRs too? I guess
> paired single (v2sf) doesn't need mentioning as it would never be
> allowed in GR_REGS so pseudos of that mode would never get ALL_REGS,
> is that correct? I.e. will we only see ALL_REGS if a particular
> pseudo/mode truly can be placed in any register according to the
> hard_regno_ok rules?
I think that with the patch below all concerns would be addressed since
the class narrowing would be constrained to integers rather than anything else.
Regards,
Robert
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index c3755f5..976f844 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19415,6 +19415,21 @@ mips_lra_p (void)
{
return mips_lra_flag;
}
+
+/* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS. */
+
+static reg_class_t
+mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
+{
+ /* LRA will generate unnecessary reloads because the LRA's cost pass finds
+ cheaper to move data to/from memory into FP regs rather than GP regs.
+ By narrowing the class for allocnos to GR_REGS for integral modes early,
+ we refrain from using FP regs until they are absolutely necessary. */
+ if (INTEGRAL_MODE_P (PSEUDO_REGNO_MODE (regno)) && allocno_class == ALL_REGS)
+return GR_REGS;
+ return allocno_class;
+}
+
/* Initialize the GCC target structure. */
#undef TARGET_ASM_ALIGNED_HI_OP
@@ -19671,6 +19686,8 @@ mips_lra_p (void)
#define TARGET_SPILL_CLASS mips_spill_class
#undef TARGET_LRA_P
#define TARGET_LRA_P mips_lra_p
+#undef TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
+#define TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
mips_ira_change_pseudo_allocno_class
struct gcc_target targetm = TARGET_INITIALIZER;
RE: [Patch MIPS] Enable TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook
Hi Matthew, > /* LRA will allocate an FPR for an integer mode pseudo instead of spilling >to memory if an FPR is present in the allocno class. It is rare that >we actually need to place an integer mode value in an FPR so where >possible limit the allocation to GR_REGS. This will slightly pessimize >code that involves integer to/from float conversions as these will have >to reload into FPRs in LRA. Such reloads are sometimes eliminated and >sometimes only partially eliminated. We choose to take this penalty >in order to eliminate usage of FPRs in code that does not use floating >point data. > >This change has a similar effect to increasing the cost of FPR->GPR >register moves for integer modes so that they are higher than the cost >of memory but changing the allocno class is more reliable. > >This is also similar to forbidding integer mode values in FPRs entirely >but this would lead to an inconsistency in the integer to/from float >instructions that say integer mode values must be placed in FPRs. */ > > I'm keen to get the description of this right so please feel free to change > it further if it isn't clear (or correct). This description is definitely more accurate. At first glance, I wasn't sure whether to include the bit about partial elimination since it caused by post reload passes but the introduction of reloads should be avoided in the first place. > I don't know if this change will lead to classic reload being unusable for > MIPS. I'm not worried about that but I think it is probably wise to > remove classic reload support for MIPS now; we are dependent on LRA for > several features already. Indeed. I think it's the right time to drop the classic reload and resolve LRA issues if they come up. > Do you have any details on when we are left with suboptimal code for > int->float conversions? I'd like to keep a record of them in this thread > or in the comment so we know what is left to fix. I opened a bug report https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66204 and just realised that I had never CCed Vlad. Hopefully, this is the last remaining issue related to integers and floating-point registers. > > > + if (INTEGRAL_MODE_P (PSEUDO_REGNO_MODE (regno)) && allocno_class == > > ALL_REGS) > > +return GR_REGS; > > + return allocno_class; > > +} > > + > > Trim the extra trailing newline. > > OK to commit if you are happy with the comment. I'll update the comment, do a quick regression and commit it. Regards, Robert
RE: [Patch MIPS] Enable TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook
Hi, > > Trim the extra trailing newline. > > OK to commit if you are happy with the comment. Committed as r224549. Regards, Robert
[PATCH, MIPS] Support interrupt handlers with hard-float
Hi Matthew/Catherine,
The attached patch removes the restriction to compile a TU with an ISR with
-mhard-float. Instead of forcing -msoft-float, the coprocessor 1 is disabled in
an ISR for -mhard-float.
Ok to apply?
Regards,
Robert
gcc/
* config/mips/mips.c (mips_compute_frame_info): Allow -mhard-float in
interrupt attribute.
(mips_expand_prologue): Disable the floating point unit in an ISR for
-mhard-float.
* config/mips/mips.h (SR_COP1): New define.
---
gcc/config/mips/mips.c | 10 --
gcc/config/mips/mips.h | 2 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index b6ad7db..18cb2bc 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -10424,8 +10424,6 @@ mips_compute_frame_info (void)
{
if (mips_isa_rev < 2)
error ("the % attribute requires a MIPS32r2 processor or
greater");
- else if (TARGET_HARD_FLOAT)
- error ("the % attribute requires %<-msoft-float%>");
else if (TARGET_MIPS16)
error ("interrupt handlers cannot be MIPS16 functions");
else
@@ -11676,6 +11674,14 @@ mips_expand_prologue (void)
GEN_INT (5),
GEN_INT (SR_IE),
gen_rtx_REG (SImode, GP_REG_FIRST)));
+
+ if (TARGET_HARD_FLOAT)
+ /* Disable COP1 for hard-float. This will lead to an exception
+ if floating-point code is executed in an ISR. */
+ emit_insn (gen_insvsi (gen_rtx_REG (SImode, K1_REG_NUM),
+ GEN_INT (1),
+ GEN_INT (SR_COP1),
+ gen_rtx_REG (SImode, GP_REG_FIRST)));
}
else
{
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 0e14f90..24cf65c 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -1807,6 +1807,8 @@ FP_ASM_SPEC "\
/* Request Interrupt Priority Level is from bit 10 to bit 15 of
the cause register for the EIC interrupt mode. */
#define CAUSE_IPL 10
+/* COP1 Enable is at bit 29 of the status register. */
+#define SR_COP1 29
/* Interrupt Priority Level is from bit 10 to bit 15 of the status register.
*/
#define SR_IPL 10
/* Interrupt masks start with IM0 at bit 8 to IM7 at bit 15 of the status
--
2.2.2
[PATCH, MIPS] Support new interrupt handler options
Hi,
This patch adds support for optional arguments for interrupt and
use_shadow_register_set attributes. The patch also fixes an ICE if both
interrupt and use_shadow_register_set are enabled and compiled with -mips64r2
-mabi=64 discovered during testing of the attached test.
The interrupt attribute accepts new arguments: "eic" and
"vector=[sw0|sw1|hw0|hw1|hw2|hw3|hw4|hw5]". The former is the default if no
argument is given and the latter changes the behaviour of GCC and masks
interrupts from sw0 up to and including the specified vector. As part of this
change, the EPC is now saved and restored unconditionally to recover the state
in nested interrupts. Only K1 register is clobbered for masked interrupts but
for non-masked interrupts K0 is still used.
The use_shadow_register_set attribute has a new option, "intstack", to indicate
that the shadow register set has a valid stack pointer. With this option
"rdpgpr $sp, $sp" will not be generated for an ISR.
Tested with mips-img-elf, mips-img-linux-gnu and mips64el-linux-gnu cross
compilers. Ok to apply?
Regards,
Robert
2015-07-07 Matthew Fortune
Robert Suchanek
gcc/
* config/mips/mips.c (mips_int_mask): New enum.
(mips_shadow_set): Likewise.
(int_mask): New variable.
(use_shadow_register_set_p): Change type to enum mips_shadow_set.
(machine_function): Add int_mask and use_shadow_register_set.
(mips_attribute_table): Add attribute handlers for interrupt and
use_shadow_register_set.
(mips_interrupt_mask): New static function.
(mips_handle_interrupt_attr): Likewise.
(mips_handle_use_shadow_register_set_attr): Likewise.
(mips_use_shadow_register_set): Change return type to enum
mips_shadow_set. Add argument handling for use_shadow_register_set
attribute.
(mips_interrupt_extra_called_saved_reg_p): Update the conditional to
compare with mips_shadow_set enum.
(mips_compute_frame_info): Add interrupt mask and
use_shadow_register_set to per-function information structure.
Add a stack slot for EPC unconditionally.
(mips_expand_prologue): Compare use_shadow_register_set value
with mips_shadow_set enum. Save EPC always in K1, clobber only K1 for
masked interrupt register but in EIC mode use K0 and save Cause in K0.
EPC saved and restored unconditionally. Use PMODE_INSN macro when
copying the stack pointer from the shadow register set.
* config/mips/mips.h (SR_IM0): New define.
* config/mips/mips.md (mips_rdpgpr): Rename to...
(mips_rdpgpr_): ...this. Use the Pmode iterator.
* doc/extend.texi (Declaring Attributes of Functions): Document
optional arguments for interrupt and use_shadow_register_set
attributes.
gcc/testsuite/
* gcc.target/mips/interrupt_handler-4.c: New test.
---
gcc/config/mips/mips.c | 276 +
gcc/config/mips/mips.h | 3 +
gcc/config/mips/mips.md| 10 +-
gcc/doc/extend.texi| 22 +-
.../gcc.target/mips/interrupt_handler-4.c | 31 +++
5 files changed, 281 insertions(+), 61 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/mips/interrupt_handler-4.c
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index ce21a0f..b6ad7db 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -390,6 +390,30 @@ struct GTY(()) mips_frame_info {
HOST_WIDE_INT hard_frame_pointer_offset;
};
+/* Enumeration for masked vectored (VI) and non-masked (EIC) interrupts. */
+enum mips_int_mask
+{
+ INT_MASK_EIC = -1,
+ INT_MASK_SW0 = 0,
+ INT_MASK_SW1 = 1,
+ INT_MASK_HW0 = 2,
+ INT_MASK_HW1 = 3,
+ INT_MASK_HW2 = 4,
+ INT_MASK_HW3 = 5,
+ INT_MASK_HW4 = 6,
+ INT_MASK_HW5 = 7
+};
+
+/* Enumeration to mark the existence of the shadow register set.
+ SHADOW_SET_INTSTACK indicates a shadow register set with a valid stack
+ pointer. */
+enum mips_shadow_set
+{
+ SHADOW_SET_NO,
+ SHADOW_SET_YES,
+ SHADOW_SET_INTSTACK
+};
+
struct GTY(()) machine_function {
/* The next floating-point condition-code register to allocate
for ISA_HAS_8CC targets, relative to ST_REG_FIRST. */
@@ -442,8 +466,12 @@ struct GTY(()) machine_function {
/* True if this is an interrupt handler. */
bool interrupt_handler_p;
- /* True if this is an interrupt handler that uses shadow registers. */
- bool use_shadow_register_set_p;
+ /* Records the way in which interrupts should be masked. Only used if
+ interrupts are not kept masked. */
+ enum mips_int_mask int_mask;
+
+ /* Records if this is an interrupt handler that uses shadow registers. */
+ enum mips_shadow_set use_shadow_register_set;
/* True if this is an interrupt handler that should keep interrupts
mas
[PATCH, MIPS] Fix restoration of hi/lo in MIPS64R2 interrupt handlers
Hi,
The attached patch fixes an ICE (unrecognizable insn) when accumulators are
used in interrupt handlers for MIPS64R2. There was just a typo in the function
name.
Ok to apply?
Regards,
Robert
gcc/
* config/mips/mips.c (mips_emit_save_slot_move): Fix typo.
gcc/testsuite/
* gcc.target/mips/20150707.c: New test.
---
gcc/config/mips/mips.c | 2 +-
gcc/testsuite/gcc.target/mips/20150707.c | 7 +++
2 files changed, 8 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/mips/20150707.c
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index ef8b0c7..1f247cf 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -10961,7 +10961,7 @@ mips_emit_save_slot_move (rtx dest, rtx src, rtx temp)
{
mips_emit_move (temp, src);
if (TARGET_64BIT)
- emit_insn (gen_mthisi_di (gen_rtx_REG (TImode, MD_REG_FIRST),
+ emit_insn (gen_mthidi_ti (gen_rtx_REG (TImode, MD_REG_FIRST),
temp, gen_rtx_REG (DImode, LO_REGNUM)));
else
emit_insn (gen_mthisi_di (gen_rtx_REG (DImode, MD_REG_FIRST),
diff --git a/gcc/testsuite/gcc.target/mips/20150707.c
b/gcc/testsuite/gcc.target/mips/20150707.c
new file mode 100644
index 000..434b3b8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/20150707.c
@@ -0,0 +1,7 @@
+/* { dg-do assemble } */
+/* { dg-options "-mips64r2" } */
+_Accum a;
+__attribute__((interrupt))
+void foo () {
+ a = a*a;
+}
--
2.2.2
[PATCH, RFC] LRA subreg handling
Hi Vladimir,
An issue has been identified with LRA when running CPU2006 h264ref benchmark.
I'll try to describe what the issue is and a fix applied as it is very
difficult to reproduce it and it is next to impossible to create a narrowed
testcase on top of the source code restrictions.
The concerned LRA code in lra-constraints.c is the following:
if (GET_CODE (*loc) == SUBREG)
{
reg = SUBREG_REG (*loc);
byte = SUBREG_BYTE (*loc);
if (REG_P (reg)
/* Strict_low_part requires reload the register not
the sub-register. */
&& (curr_static_id->operand[i].strict_low
|| (GET_MODE_SIZE (mode)
<= GET_MODE_SIZE (GET_MODE (reg))
&& (hard_regno
= get_try_hard_regno (REGNO (reg))) >= 0
&& (simplify_subreg_regno
(hard_regno,
GET_MODE (reg), byte, mode) < 0)
&& (goal_alt[i] == NO_REGS
|| (simplify_subreg_regno
(ira_class_hard_regs[goal_alt[i]][0],
GET_MODE (reg), byte, mode) >= 0
{
loc = &SUBREG_REG (*loc);
mode = GET_MODE (*loc);
}
}
The above works just fine when we deal with strict_low_part or a subreg
smaller than a word.
However, multi-word operations that were emitted as a sequence of operations
on word sized parts of the DImode register appears to expose a problem with
LRA e.g. '(set (subreg: SI (reg: DI)) ...)'.
LRA does not realize that it actually uses the other halve of the DI-mode
register leading to a situation where it modifies one halve of the result and
spills the whole register with the other halve undefined.
In the dump I can see the following:
Creating newreg=1552 from oldreg=521, assigning class GR_REGS to r1552
1487: r1552:DI#4=r1404:SI+r1509:SI
REG_DEAD r1509:SI
REG_DEAD r1404:SI
Inserting insn reload after:
1735: r521:DI=r1552:DI
There is nothing in the dump that sets r1552:DI#0 nor a reload is inserted
to load the value before modifying it but it is spilled.
As it is a multi-word register, the split pass emits an additional instruction
to load the whole 64-bit value but since one halve was modified, only
register $20 appears in the live-in set. In contrast to $20, $21 is being used
but not added to the live-in set.
...
;; live in 4 [$4] 6 [$6] 7 [$7] 10 [$10] 11 [$11] 12 [$12] 13 [$13]
[$14] 15 [$15] 16 [$16] 17 [$17] 20 [$20] 22 [$22] 23 [$23] 24 [$24] 25 [$25]
29 [$sp] 30 [$fp] 31 [$31] 52 [$f20] 79 [$fakec]
...
(insn 1788 1077 1789 80 (set (reg:SI 20 $20 [orig:521 distortion ] [521])
(mem/c:SI (plus:SI (reg/f:SI 29 $sp)
(const_int 40 [0x28])) [16 %sfp+40 S4 A64])) rdopt.c:257 288
{*movsi_internal}
(nil))
(insn 1789 1788 1743 80 (set (reg:SI 21 $21 [ distortion+4 ])
(mem/c:SI (plus:SI (reg/f:SI 29 $sp)
(const_int 44 [0x2c])) [16 %sfp+44 S4 A32])) rdopt.c:257 288
{*movsi_internal}
(nil))
...
The potential fix for this is to promote the type of a subreg OP_OUT to OP_INOUT
to treat the pseudo register (r1552 in this case) as input and LRA will be
forced
to insert a reload before modifying its contents.
Handling of strict_low_part case is fine as the operand is described in the MD
pattern
as IN_OUT through modifiers.
With the above change in place, we get a reload before assignment:
Creating newreg=1552 from oldreg=521, assigning class GR_REGS to r1552
1487: r1552:DI#4=r1404:SI+r1509:SI
REG_DEAD r1509:SI
REG_DEAD r1404:SI
Inserting insn reload before:
1735: r1552:DI=r521:DI
Inserting insn reload after:
1736: r521:DI=r1552:DI
and the benchmark happily passes the runtime check.
The question is whether changing the type to OP_INOUT is the correct and
valid fix?
Regards,
Robert
2015-01-14 Robert Suchanek
gcc/
* lra-constraints.c (curr_insn_transform): Change the type of a reload
pseudo to OP_INOUT.
---
gcc/lra-constraints.c |1 +
1 file changed, 1 insertion(+)
diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index ec28b7f..018968b 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -3798,6 +3798,7 @@ curr_insn_transform (void)
(ira_class_hard_regs[goal_alt[i]][0],
GET_MODE (reg), byte, mode) >= 0)
{
+ type = OP_INOUT;
loc = &SUBREG_REG (*loc);
mode = GET_MODE (*loc);
}
--
1.7.9.5
RE: [PATCH, RFC] LRA subreg handling
> Robert, can you look at reload.c::reload_inner_reg_of_subreg and verify > that the comment just before its return statement is effectively the > situation you're in. > > There are certainly cases where a SUBREG needs to be treated as an > in-out operand. We walked through them eons ago when we were poking at > SSA for RTL. But the details have long since faded from memory. The comment pretty much applies to my situation. The only difference I can see is that reload would have had hard registers at this point. In the testcase, LRA does not have hard registers assigned to the concerned pseudo(s), thus, it can't rely on the information in hard_regno_nregs to check if the number of registers in INNER is different to the number of words in INNER. Robert
RE: [RFC, PATCH][LRA, MIPS] ICE: in decompose_normal_address, at rtlanal.c:5817
> OK. The MIPS and Sparc ports are probably going to hit this the > hardest. So you've got a vested interest in dealing with any fallout :-) > > jeff That's fine. The MIPS port has been widely tested and I cross tested it on sparc-linux-gnu target so hopefully there won't any fallout. Robert
RE: [PATCH, RFC] LRA subreg handling
> The differences (hard vs pseudo regs) are primarily an implementation > detail. I was really looking to see if there was existing code which > would turn an output reload into an in-out reload for these subregs. > > The in-out nature of certain subregs is something I've personally > stumbled over in various contexts (for example, this also came up during > RTL-SSA investigations years ago). > > Jeff Although I haven't seen an output reload changing to in-out reload in the testcase after analysing the reload code I'm inclined to say the change of the out to in-out reload can happen for these subregs in push_reload(). Robert
[PATCH RFA MIPS] Prohibit vector modes in accumulators
Hi,
A bug was exposed by LRA for loongson-shift-count-truncated-1.c and
loongson-simd.c with -O0 optimization. These testcases were ICEing
with 'Max. number of generated reloads insns per insn is achieved (90)'
error.
The problem appears to be with vector modes where contents of memory
is meant to be copied into GPRs but LRA chose to put them into accumulators
first and started inserting reloads to move the values into GPRs.
In both cases, IRA assigned the alternative GR_AND_MD0_REGS class to
allocnos. Since accumulators appear first in REG_ALLOC_ORDER macro,
LRA picks LO/HI registers even if it's more costly than GPRs and wants
to insert reloads to move in/out values to/from accumulators for which
we don't have patterns i.e. to handle (set (reg:V2SI ) (mem:V2SI ...)).
The fix is to prohibit the use of accumulator registers for vector modes.
Unfortunately, I have no testcase exposing this on the trunk but it seems
reasonable to prohibit vectors in accumulators anyway.
I'm not sure if this patch would go to the trunk now and be queued for
Stage 1.
Regards,
Robert
2015-01-23 Robert Suchanek
* config/mips/mips.c (mips_hard_regno_mode_ok_p): Prohibit accumulators
for all vector modes.
---
gcc/config/mips/mips.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 443a712..1733457 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -12131,7 +12131,9 @@ mips_hard_regno_mode_ok_p (unsigned int regno,
machine_mode mode)
return size >= MIN_UNITS_PER_WORD && size <= UNITS_PER_FPREG;
}
+ /* Don't allow vector modes in accumulators. */
if (ACC_REG_P (regno)
+ && !VECTOR_MODE_P (mode)
&& (INTEGRAL_MODE_P (mode) || ALL_FIXED_POINT_MODE_P (mode)))
{
if (MD_REG_P (regno))
--
2.2.2
RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators
Hi Catherine, > The patch looks reasonable, but I'd like to see a test case that fails before > we agree to include for GCC 5.0. It's possible to reproduce ICEs with SVN revision 212354 on mipsel-linux-gnu target. The concerned tests are loongson-simd.c and loongson-shift-count-truncated-1.c in gcc/testsuite/gcc.target/mips. mipsel-linux-gcc -O0 -march=loongson2f loongson-shift-count-truncated-1.c Regards, Robert
[PATCH MIPS RFA] Regression cleanup for nan2008 toolchain
Hi,
Here is a patch to clean up a large number of reported failures when a toolchain
is configured with --with-nan=2008 for mips*-linux-gnu triplet and clean up a
failures
for mips-img-linux-gnu where nan2008 is set by the default.
In the former case, regression involves testing e.g. -mips4 with default
options which
causes emission of warnings that the -mips4 architecture does not support
nan2008, and
hence, the test is classified as a fail ("test for excess errors" type of
error).
The patch implies -mnan=legacy switch for all tests that are run for ISA rev <
2.
In the latter case, even if we decrease the ISA level for incompatible options
for
the loongson vector extension tests, especially loongson-simd.c, a test that
includes
stdint.h or stdlib.h will end up including the glibc header 'stubs-.h>'
and will
fail as the mipsr6 toolchain will only have stubs-o32_hard_2008.h. A toolchain
supporting
nan1985 will have the required stubs-o32_hard.h. The only neat solution AFAICS
was to add
a new effective target that tries to compile and include stdlib.h to check if
we have
the proper support for the legacy NaN marking the concerned tests as
UNSUPPORTED.
Regards,
Robert
2015-01-26 Robert Suchanek
gcc/testsuite
* lib/target-supports.exp (check_effective_target_mips_nanlegacy): New.
* gcc.target/mips/loongson-simd.c: Require legacy NaN support.
* gcc.target/mips/mips.exp (mips-dg-options): Imply -mnan=legacy for
ISA rev < 2.
---
gcc/testsuite/gcc.target/mips/loongson-simd.c |1 +
gcc/testsuite/gcc.target/mips/mips.exp|1 +
gcc/testsuite/lib/target-supports.exp |9 +
3 files changed, 11 insertions(+)
diff --git a/gcc/testsuite/gcc.target/mips/loongson-simd.c
b/gcc/testsuite/gcc.target/mips/loongson-simd.c
index 160da6b..949632e 100644
--- a/gcc/testsuite/gcc.target/mips/loongson-simd.c
+++ b/gcc/testsuite/gcc.target/mips/loongson-simd.c
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3. If not see
/* loongson.h does not handle or check for MIPS16ness or
microMIPSness. There doesn't seem any good reason for it to, given
that the Loongson processors do not support either. */
+/* { dg-require-effective-target mips_nanlegacy } */
/* { dg-options "isa=loongson -mhard-float -mno-micromips -mno-mips16
-flax-vector-conversions" } */
#include "loongson.h"
diff --git a/gcc/testsuite/gcc.target/mips/mips.exp
b/gcc/testsuite/gcc.target/mips/mips.exp
index b81d344..a0980a9 100644
--- a/gcc/testsuite/gcc.target/mips/mips.exp
+++ b/gcc/testsuite/gcc.target/mips/mips.exp
@@ -1300,6 +1300,7 @@ proc mips-dg-options { args } {
mips_make_test_option options "-mno-dsp"
mips_make_test_option options "-mno-synci"
mips_make_test_option options "-mno-micromips"
+ mips_make_test_option options "-mnan=legacy"
}
if { $isa_rev > 5 } {
mips_make_test_option options "-mno-dsp"
diff --git a/gcc/testsuite/lib/target-supports.exp
b/gcc/testsuite/lib/target-supports.exp
index e51d07d..de2f599 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3036,6 +3036,15 @@ proc check_effective_target_mips_loongson { } {
}]
}
+# Return 1 if this is a MIPS target that supports the legacy NAN.
+
+proc check_effective_target_mips_nanlegacy { } {
+return [check_no_compiler_messages nanlegacy assembly {
+ #include
+ int main () { return 0; }
+} "-mnan=legacy"]
+}
+
# Return 1 if this is an ARM target that adheres to the ABI for the ARM
# Architecture.
--
1.7.9.5
RE: [PATCH, RFC] LRA subreg handling
> > Here we do have a hard register, but it isn't valid to form the subreg > > on that hard register. Reload had to cope with that case too. > > > > Since the subreg on the original hard register is invalid, we can't use > > it to decide whether the intention was to write to only a part of the > > inner register. But I don't think we need to use the hard register > > here. The original register was a psuedo and I'm pretty sure... Indeed, it is a hard register (64 IIRC) since get_try_hard_regno () must return >= 0. > > > The differences (hard vs pseudo regs) are primarily an implementation > > > detail. I was really looking to see if there was existing code which > > > would turn an output reload into an in-out reload for these subregs. > > > > > > The in-out nature of certain subregs is something I've personally > > > stumbled over in various contexts (for example, this also came up > > > during RTL-SSA investigations years ago). > > > > ...the rule for pseudos is that words of a multiword pseudo can be > > accessed independently but subword pieces of an individual word can't. > > This obviously isn't ideal if a mode is intended for wider-than-word > > registers, since not all words will be independently addressable when > > allocated to those registers. The code above is partly dealing with the > > fallout from that. It's also why we have strict_lowpart for cases like > > al on i386. > > > > So IMO the patch is too broad. I think it should only use INOUT reloads > > for !strict_low if the inner mode is wider than a word and the outer > > mode is strictly narrower than the inner mode. That's on top of Vlad's > > comment about only converting OP_OUTs, of course. > > This sounds correct/sensible. The change as committed will be turning > some OP_OUT reloads into OP_INOUT unnecessarily. Checking for !strict_low > is however probably redundant as strict_low must be OP_INOUT and never > OP_OUT but we could mask backend bugs by converting strict_low subregs. > > I think this should be resolved for GCC 5 if others agree it is an issue. > > Matthew This makes sense since we've got the inner wider than a word and I agree it'd be better to resolve this for GCC 5. Robert
[PATCH RFC] Running auto-vectorization tests multiple times
Hi,
I'm trying to lift the restriction to run auto-vectorization tests
more than once and would like to check if I'm going in the right
direction. I attached a draft patch.
Currently, auto-vectorization tests are enabled by a call to
check_vect_support_and_set_flags procedure and if there is support
then the global DEFAULT_VECTFLAGS is modified and the procedure
returns true. However, this is problematic in the MIPS case as we
have Loongson vector support, Paired-Single (PS) instructions and
the new MIPS SIMD ASE (MSA). Loongson is the easiest as it cannot
be executed along with the options for PS and MSA (both require -mfp64)
and vice versa, but we can vectorise PS and MSA in a single run.
This obviously requires a possibility to run the same tests twice,
with -mpaired-single and -mmsa.
check_vect_support_and_set_flags is called in a few places:
- gcc.dg/graphite/graphite.exp
- gcc.dg/vect/vect.exp
- gfortran.dg/graphite/graphite.exp
- gfortran.dg/vect/vect.exp
- g++.dg/vect/vect.exp
The first complication is the use of variations of dg-runtest:
gcc-dg-runtest, g++-dg-runtest, gfortran-dg-runtest and the plain
dg-runtest.
The idea to enable multi execution is to add a new procedure,
et-dg-runtest, with similar arguments as the above but with
an additional argument taking the name of procedure for a callback.
et-dg-runtest would iterate over a list of all effective targets
i.e. global ET_TARGETS, setup by check_vect_support_and_set_flags,
add needed options for a target and check if we want to just compile
or run the tests by calling helpers.
Implementation would require a couple of helper procedures to make
it work, thus, for each e.g. mips_loongson, mips_msa we would
define the following procedures in lib/target-supports.exp:
- add_options_for_ (optional) returning flags needed for
- check_effective_target_ to check if we can compile
- check_effective_target__hw_available to check if we can run
- check_effective_target__runtime (optional) - calls the two
above plus any other additional checks like OS environment, etc.
If a port wants to exploit multiple execution then would
need to be appended in vect_support_and_set_flags to the global
ET_TARGETS and add the aforementioned helper procedures.
Some ports already follow this convention to certain extent. However,
DEFAULT_VECTCFLAGS should not be set as the appropriate flags will
be added on the fly via add_options_for_ if it's defined.
If the port supports only one vector extension, no other changes are needed
and DEFAULT_VECTCFLAGS should be modified as per old behaviour.
Any other thoughts/suggestions?
The patch is a concept and will not run MSA tests as the MSA support
is not in the trunk yet.
Regards,
Robert
[PATCH] Run vector tests for more than one effective target.
gcc/testsuite
* g++.dg/vect/vect.exp: Add and set new global ET_TARGETS. Call
g++-dg-runtest via et-dg-runtest.
* gcc.dg/graphite/graphite.exp: Likewise, but for gcc-dg-runtest.
* gcc.dg/vect/vect.exp: Likewise.
* gfortran.dg/graphite/graphite.exp: Likewise, but for
gfortran-dg-runtest.
* gfortran.dg/vect/vect.exp: Likewise.
* lib/target-supports.exp (check_mpaired_single_hw_available): New.
(check_mips_loongson_hw_available): Likewise.
(check_mips_msa_hw_available): Likewise.
(check_effective_target_mpaired_single_runtime): Likewise.
(check_effective_target_mips_loongson_runtime): Likewise.
(check_effective_target_mips_msa_runtime): Likewise.
(add_options_for_mpaired_single): Likewise.
(add_options_for_mips_msa): Likewise.
(check_effective_target_mips_msa): Likewise.
(et-dg-runtest): Likewise.
(vect_support_and_set_flags): Add supported MIPS targets to ET_TARGETS
list.
---
gcc/testsuite/g++.dg/vect/vect.exp | 16 +-
gcc/testsuite/gcc.dg/graphite/graphite.exp |6 +-
gcc/testsuite/gcc.dg/vect/vect.exp | 157 +++---
gcc/testsuite/gfortran.dg/graphite/graphite.exp |7 +-
gcc/testsuite/gfortran.dg/vect/vect.exp | 45 --
gcc/testsuite/lib/target-supports.exp | 195 ++-
6 files changed, 340 insertions(+), 86 deletions(-)
diff --git a/gcc/testsuite/g++.dg/vect/vect.exp
b/gcc/testsuite/g++.dg/vect/vect.exp
index aba1866..1e02d6b 100644
--- a/gcc/testsuite/g++.dg/vect/vect.exp
+++ b/gcc/testsuite/g++.dg/vect/vect.exp
@@ -39,6 +39,10 @@ set save-dg-do-what-default ${dg-do-what-default}
global DEFAULT_VECTCFLAGS
set DEFAULT_VECTCFLAGS ""
+# Reset list of effective targets to be checked against tests.
+global ET_TARGETS
+set ET_TARGETS ""
+
# These flags are used for all targets.
lappend DEFAULT_VECTCFLAGS "-O2" "-ftree-vectorize" "-fno-vect-cost-model"
@@ -58,10 +62,10 @@ lappend VECT_SLP_CFLAGS "-fdump-tree-slp-details"
dg-init
# Main loop.
-g++-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/pr*.{c,cc,S} ]] \
-
RE: [PATCH MIPS RFA] Regression cleanup for nan2008 toolchain
> > > > 2015-01-26 Robert Suchanek > > > > gcc/testsuite > > * lib/target-supports.exp (check_effective_target_mips_nanlegacy): > > New. > > * gcc.target/mips/loongson-simd.c: Require legacy NaN support. > > * gcc.target/mips/mips.exp (mips-dg-options): Imply -mnan=legacy > > for > > ISA rev < 2. > > --- > > This patch is OK. Committed as 220199. Regards, Robert
RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators
> > Since Catherine asked for further info then I will leave her to say if she > is > > happy to accept on this basis. > > > > I withdraw my request for a testcase. > > Catherine Committed as r220200. Regards, Robert
RE: [PATCH MIPS RFA] Regression cleanup for nan2008 toolchain
> Please could you add a comment explaining that the mips_nanlegacy is there
> because of the #include of system headers that might not compile with
> -mnan=legacy? I agree that that's a good reason, but it's not obvious
> without a comment. (And without a comment this could start a precendent
> of things being skipped in cases where the mips.exp options machinery
> could be updated instead.)
>
True. Clarification added.
Ok for trunk?
Regards,
Robert
2015-02-02 Robert Suchanek
* gcc.target/mips/loongson-simd.c: Update comment to clarify the need
for mips_nanlegacy target.
diff --git a/gcc/testsuite/gcc.target/mips/loongson-simd.c
b/gcc/testsuite/gcc.target/mips/loongson-simd.c
index 949632e..9c3ebce 100644
--- a/gcc/testsuite/gcc.target/mips/loongson-simd.c
+++ b/gcc/testsuite/gcc.target/mips/loongson-simd.c
@@ -21,7 +21,10 @@ along with GCC; see the file COPYING3. If not see
/* { dg-do run } */
/* loongson.h does not handle or check for MIPS16ness or
microMIPSness. There doesn't seem any good reason for it to, given
- that the Loongson processors do not support either. */
+ that the Loongson processors do not support either. The effective target
+ mips_nanlegacy is required for a toolchain without the legacy NaN support
+ because inclusion of some system headers e.g. stdint.h will fail due to not
+ finding stubs-o32_hard.h. */
/* { dg-require-effective-target mips_nanlegacy } */
/* { dg-options "isa=loongson -mhard-float -mno-micromips -mno-mips16
-flax-vector-conversions" } */
RE: [PATCH MIPS RFA] Regression cleanup for nan2008 toolchain
> > 2015-02-02 Robert Suchanek
> >
> >* gcc.target/mips/loongson-simd.c: Update comment to clarify the
> need
> >for mips_nanlegacy target.
> >
> > diff --git a/gcc/testsuite/gcc.target/mips/loongson-simd.c
> > b/gcc/testsuite/gcc.target/mips/loongson-simd.c
> > index 949632e..9c3ebce 100644
> > --- a/gcc/testsuite/gcc.target/mips/loongson-simd.c
> > +++ b/gcc/testsuite/gcc.target/mips/loongson-simd.c
> > @@ -21,7 +21,10 @@ along with GCC; see the file COPYING3. If not see
> > /* { dg-do run } */
> > /* loongson.h does not handle or check for MIPS16ness or
> > microMIPSness. There doesn't seem any good reason for it to, given
> > - that the Loongson processors do not support either. */
> > + that the Loongson processors do not support either. The effective
> target
> > + mips_nanlegacy is required for a toolchain without the legacy NaN
> support
> > + because inclusion of some system headers e.g. stdint.h will fail due to
> not
> > + finding stubs-o32_hard.h. */
> > /* { dg-require-effective-target mips_nanlegacy } */
> > /* { dg-options "isa=loongson -mhard-float -mno-micromips -mno-mips16 -
> > flax-vector-conversions" } */
> >
>
> Thanks for the update. This is OK.
> Catherine
Committed as r220393.
Thanks and regards,
Robert
RE: [PATCH 3/4] Add support to run auto-vectorization tests for multiple effective targets
Hi,
> unfortunately this broke make check-c
> RUNTESTFLAGS='vect.exp=*no-vfa-vect-dv-2.c
> --target_board=unix\{-m32,-m64\}', causing the check if
> vect_aligned_arrays to be cached between the -m64 and -m32 variants
> which is incorrect at least on my machine if you actually run that test
> for -m32 and -m64 you get different results. In both case et_index is 0
> so you use the cached value the second time, but that's not correct
> because the options changed.
>
> I suspect this also causes some random vectorizer tests to appear and
> disappear during regression testing with the same -m64 and -m32, but I'm
> not absolutely sure of that part.
>
> Thanks!
>
> Trev
I was misled by the comments in a few procedures suggesting that the results
should have been cached and the use of global variable looked mistyped.
The following patch reverts to the old behaviour. I also removed misleading
comments and related logic that checks for the cached result. There might be
other procedures with similar inconsistency but here I only modified the
offending ones.
Alternatively, it would be possible to switch to the new method and do the
caching
but it is more intrusive change that requires careful analysis of the results
and the tests will not likely be directly comparable with old results (because
of
flag mixing into the test names). It's safer to restore the original behaviour
as the patch was not supposed to change any existing results.
Ok to apply?
Regards,
Robert
gcc/testsuite/
* lib/target-supports.exp
(check_effective_target_vect_aligned_arrays): Don't cache the result.
(check_effective_target_vect_natural_alignment): Ditto.
(check_effective_target_vector_alignment_reachable): Ditto.
(check_effective_target_vector_alignment_reachable_for_64bit): Ditto.
diff --git a/gcc/testsuite/lib/target-supports.exp
b/gcc/testsuite/lib/target-supports.exp
index 533d3a6..0dabea0 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -5309,32 +5309,22 @@ proc check_effective_target_vect_hw_misalign { } {
# Return 1 if arrays are aligned to the vector alignment
# boundary, 0 otherwise.
-#
-# This won't change for different subtargets so cache the result.
proc check_effective_target_vect_aligned_arrays { } {
-global et_vect_aligned_arrays_saved
-global et_index
-
-if [info exists et_vect_aligned_arrays_saved($et_index)] {
- verbose "check_effective_target_vect_aligned_arrays:\
-using cached result" 2
-} else {
- set et_vect_aligned_arrays_saved($et_index) 0
-if { ([istarget x86_64-*-*] || [istarget i?86-*-*]) } {
- if { ([is-effective-target lp64]
- && ( ![check_avx_available]
-|| [check_prefer_avx128])) } {
-set et_vect_aligned_arrays_saved($et_index) 1
- }
- }
-if [istarget spu-*-*] {
- set et_vect_aligned_arrays_saved($et_index) 1
+set et_vect_aligned_arrays 0
+if { ([istarget x86_64-*-*] || [istarget i?86-*-*]) } {
+ if { ([is-effective-target lp64]
+ && ( ![check_avx_available]
+|| [check_prefer_avx128])) } {
+set et_vect_aligned_arrays 1
}
}
+if [istarget spu-*-*] {
+ set et_vect_aligned_arrays 1
+}
verbose "check_effective_target_vect_aligned_arrays:\
-returning $et_vect_aligned_arrays_saved($et_index)" 2
-return $et_vect_aligned_arrays_saved($et_index)
+returning $et_vect_aligned_arrays" 2
+return $et_vect_aligned_arrays
}
# Return 1 if types of size 32 bit or less are naturally aligned
@@ -5382,74 +5372,43 @@ proc check_effective_target_natural_alignment_64 { } {
# Return 1 if all vector types are naturally aligned (aligned to their
# type-size), 0 otherwise.
-#
-# This won't change for different subtargets so cache the result.
proc check_effective_target_vect_natural_alignment { } {
-global et_vect_natural_alignment_saved
-global et_index
-
-if [info exists et_vect_natural_alignment_saved($et_index)] {
-verbose "check_effective_target_vect_natural_alignment: using cached
result" 2
-} else {
- set et_vect_natural_alignment_saved($et_index) 1
-if { [check_effective_target_arm_eabi]
-|| [istarget nvptx-*-*]
-|| [istarget s390*-*-*] } {
- set et_vect_natural_alignment_saved($et_index) 0
-}
+set et_vect_natural_alignment 1
+if { [check_effective_target_arm_eabi]
+|| [istarget nvptx-*-*]
+|| [istarget s390*-*-*] } {
+ set et_vect_natural_alignment 0
}
verbose "check_effective_target_vect_natural_alignment:\
-returning $et_vect_natural_alignment_saved($et_index)" 2
-return $et_vect_natural_alignment_saved($et_index)
+returning $et_vect_natural_alignment" 2
+return $et_vect_natural_alignment
}
# Retu
RE: [PATCH 3/4] Add support to run auto-vectorization tests for multiple effective targets
> On 08/23/2016 04:15 PM, Trevor Saunders wrote: > > > > I've certainly been tempted to take a stab at at least replacing the > > expect stuff with something else, it drives me kind of crazy to see how > > much testsuite time is spent running expect. Even if we can't do all of > > it, the vast majority is just run the compiler and grep dump files or > > the compiler's stdout / stderr. > Yup. When Rob was making noise about rewriting dejagnu I tried to steer > him a bit towards instead replacing the expect layer, but his plans > where more on the tcl/dejagnu side. I don't think anything ever came > out of that proposal. > > Expect is severe overkill for what we need and yes, it burns an amazing > about of CPU time for what get out of it. > > Jeff True. I wasn't considering replacing the expect layer, just to overcome the limitation of auto-vect tests. This in itself is rather another hack but didn't have a suggestion what to replace it with. Regards, Robert
RE: [PATCH 3/4] Add support to run auto-vectorization tests for multiple effective targets
Hi Jeff, > > The following patch reverts to the old behaviour. I also removed misleading > > comments and related logic that checks for the cached result. There might > > be > > other procedures with similar inconsistency but here I only modified the > offending ones. > Thanks. Given how much cut-n-paste we do with the tcl code there's a > good chance this problem exists elsewhere. I don't start ranting about > tcl/dejagnu, I'll just put me in a terrible mood for the rest of the day. Indeed. > > Alternatively, it would be possible to switch to the new method and do the > caching > > but it is more intrusive change that requires careful analysis of the > > results > > and the tests will not likely be directly comparable with old results > (because of > > flag mixing into the test names). It's safer to restore the original > behaviour > > as the patch was not supposed to change any existing results. > I'd go with the safer approach for now -- I keep holding out hope that > there's a silver bullet out there that will allow us to replace dejagnu > and all its tcl goop some day. I noticed over time that tcl/dejagnu have a unique set of features. Replacing them with something else at some point wouldn't be a bad thing. > > > > > Ok to apply? > > > > Regards, > > Robert > > > > gcc/testsuite/ > > * lib/target-supports.exp > > (check_effective_target_vect_aligned_arrays): Don't cache the result. > > (check_effective_target_vect_natural_alignment): Ditto. > > (check_effective_target_vector_alignment_reachable): Ditto. > > (check_effective_target_vector_alignment_reachable_for_64bit): Ditto. > OK. > jeff > > Committed as r239730. Regards, Robert
RE: [PATCH][MIPS] Add support for code_readable function attribute
Hi Sandra,
> > +@item code_readable
> > +@cindex @code{code_readable} function attribute, MIPS
> > +For MIPS targets that support PC-relative addressing modes, this attribute
> > +can be used to control how an object is addressed. The attribute takes
> > +a single optional argument:
>
> The problem here is that we don't tell users that the argument has to be
> a string constant in quotes, and not just a token.
>
> How about changing the above text to end with:
>
> "...a single optional argument, which must be one of the following
> string constants:"
Indeed. I'll include this change.
>
> and then changing this to be @table @code and quoting the @item strings:
Likewise.
Regards,
Robert
RE: [PATCH][MIPS] Add -minline-intermix to ignore compression flags when inlining
Hi Sandra,
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index 73f1cb6..2f6195e 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -837,6 +837,7 @@ Objective-C and Objective-C++ Dialects}.
> > -mips16 -mno-mips16 -mflip-mips16 @gol
> > -minterlink-compressed -mno-interlink-compressed @gol
> > -minterlink-mips16 -mno-interlink-mips16 @gol
> > +-minline-intermix -mno-inline-intermix @gol
>
> Funky indentation here
I'm not sure what's wrong here, it's the diff I'd say. There is no
indentation within "MIPS Options".
>
> > -mabi=@var{abi} -mabicalls -mno-abicalls @gol
> > -mshared -mno-shared -mplt -mno-plt -mxgot -mno-xgot @gol
> > -mgp32 -mgp64 -mfp32 -mfpxx -mfp64 -mhard-float -msoft-float @gol
> > @@ -17916,6 +17917,18 @@ Aliases of @option{-minterlink-compressed} and
> > @option{-mno-interlink-compressed}. These options predate the microMIPS
> ASE
> > and are retained for backwards compatibility.
> >
> > +@item -minline-intermix
> > +@itemx -mno-inline-intermix
> > +@opindex minline-intermix
> > +@opindex mno-inline-intermix
> > +Enable inlining of functions which have opposing compression flags e.g.
> > +@code{mips16}/@code{nomips16} attributes.
> > +This is useful when using the @code{mips16} attribute to balance code size
> > +and performance so that a function will be compressed when not inlined or
> > +vice-versa. When using this option it is necessary to protect functions
> > +that cannot be compiled as MIPS16 with a @code{noinline} attribute to
> > ensure
> > +they are not inlined into a MIPS16 function.
>
> This flag applies to microMIPS inlining, too, right?
That's correct.
> It's confusing to only mention MIPS16.
Indeed. The option originated from MIPS16 but it can be applied to microMIPS
and it's better to mention it.
>
> Maybe you could say something like this instead:
>
> Allow inlining even if the compression flags differ between caller and
> callee. This is useful in conjunction with the @code{mips16},
> @code{micromips}, or @code{nocompression} function attributes. The code
> for the inlined function is compiled using the compression flags for the
> callee, so you may need to use the @code{noinline} attribute on
> functions that must be compiled with particular compression settings.
This sounds better.
Thanks and regards,
Robert
RE: [PATCH][MIPS] P5600 scheduler fix
Hi, > > gcc/ > > * config/mips/p5600.md (p5600_fpu_fadd): Remove checking for > > `fabs' and `fneg' type attributes. > > (p5600_fpu_fabs): Add `fmove' to the comment. > > OK. > > Thanks, > Matthew Committed as r237173. Regards, Robert
RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture
Hi, > > I'm happy to include this. Ok to commit with this change? > > This looks like it got lost at some point. I think this is a reasonable > change for safety. > > Go ahead and commit. > > Thanks, > Matthew Committed as r240965. Regards, Robert
RE: [PATCH 4/4] [MIPS] Add tests for MSA
Hi,
> -Original Message-
> From: Matthew Fortune
> Sent: 19 September 2016 15:46
> To: Robert Suchanek; catherine_mo...@mentor.com
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH 4/4] [MIPS] Add tests for MSA
>
> Hi Robert,
>
> Sorry for the long delay. Just a couple of cleanup bits in here but
> otherwise OK. Unless you want me to read through again please go
> ahead and commit once you've addressed the comments.
>
> We could do more in mips.exp for inferring -mfp64 and -mhard-float
> but I'm not sure it is worth it currently. Let's leave it for a
> round of testsuite changes at a later date.
Will do.
>
> Thanks,
> Matthew
>
> >---
> > gcc/testsuite/gcc.dg/vect/slp-26.c |7 +-
> > gcc/testsuite/gcc.dg/vect/tree-vect.h|2 +
> > gcc/testsuite/gcc.target/mips/mips.exp |8 +
> > gcc/testsuite/gcc.target/mips/msa-builtins.c | 1086
> ++
> > gcc/testsuite/gcc.target/mips/msa.c | 631 +++
> > gcc/testsuite/lib/target-supports.exp| 191 -
> > 6 files changed, 1895 insertions(+), 30 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.target/mips/msa-builtins.c
> > create mode 100644 gcc/testsuite/gcc.target/mips/msa.c
> >
> >diff --git a/gcc/testsuite/gcc.dg/vect/slp-26.c
> b/gcc/testsuite/gcc.dg/vect/slp-26.c
> >index 8b224ff..01f4e4e 100644
> >--- a/gcc/testsuite/gcc.dg/vect/slp-26.c
> >+++ b/gcc/testsuite/gcc.dg/vect/slp-26.c
> >@@ -46,6 +46,7 @@ int main (void)
> > return 0;
> > }
> >
> >-/* { dg-final { scan-tree-dump-times "vectorized 0 loops" 1 "vect" } } */
> >-/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect"
> } } */
> >-
> >+/* { dg-final { scan-tree-dump-times "vectorized 0 loops" 1 "vect" { target
> >{
> ! mips_msa } } } } */
> >+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target
> >{
> mips_msa } } } } */
> >+/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect"
> >{
> target { ! mips_msa } } } } */
> >+/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect"
> >{
> target { mips_msa } } } } */
>
> Why can we vectorise this with MSA but presumably no other arch can?
AFAICT, the reason being is that MSA supports integer division in hardware,
thus,
we can vectorize the loop.
>
> >diff --git a/gcc/testsuite/gcc.target/mips/mips.exp
> b/gcc/testsuite/gcc.target/mips/mips.exp
> >index 3258105..7c24140 100644
> >--- a/gcc/testsuite/gcc.target/mips/mips.exp
> >+++ b/gcc/testsuite/gcc.target/mips/mips.exp
> >@@ -290,6 +290,7 @@ foreach option {
> > relax-pic-calls
> > mcount-ra-address
> > odd-spreg
> >+msa
> > } {
> > lappend mips_option_groups $option "-m(no-|)$option"
> > }
> >@@ -820,6 +821,12 @@ proc mips-dg-init {} {
> > "-mno-synci",
> > #endif
> >
> >+#ifdef __mips_msa
> >+"-mmsa"
> >+#else
> >+"-mno-msa"
> >+#endif
> >+
> > 0
> > };
> > }]
> >@@ -1368,6 +1375,7 @@ proc mips-dg-options { args } {
> > mips_option_dependency options "-mfpxx" "-mno-paired-single"
> > mips_option_dependency options "-msoft-float" "-mno-paired-single"
> > mips_option_dependency options "-mno-paired-single" "-mno-mips3d"
> >+mips_option_dependency options "-mmsa" "-mno-mips16"
> >
> > # If the test requires an unsupported option, change run tests
> > # to link tests.
> >diff --git a/gcc/testsuite/gcc.target/mips/msa-builtins.c
> b/gcc/testsuite/gcc.target/mips/msa-builtins.c
> >new file mode 100644
> >index 000..f5e1efa
> >--- /dev/null
> >+++ b/gcc/testsuite/gcc.target/mips/msa-builtins.c
> >@@ -0,0 +1,1086 @@
> >+/* Test builtins for MIPS MSA ASE instructions */
> >+/* { dg-do compile } */
> >+/* { dg-options "-mfp64 -mhard-float -mmsa" } */
> >+/* { dg-skip-if "needs asm output" { *-*-* } { "-flto" } { "-ffat-lto-
> objects" } } */
>
> This should not be necessary. The scan-assembler directive forces fat-lto-
> objects
> via some higher level code. I have seen this fail at times but never tracked
> it
> down
> please remove dg-skip-if and see if you still get any -fno-fat-lto-objects
> builds
> of this file.
I placed it temporarily to work around occasional failures with the bare-metal
toolchain. As discussed, this is going to be fixed so I removed these from the
tests.
>
> >+
> >+/* { dg-final { scan-assembler "msa_addv_b.*:.*addv\\.b.*msa_addv_b" } } */
>
> Should these be seen only once: I.e. scan-assembler-times?
I changed the test to scan-assembler-times with some additional changes to
fit the correct matching and counting.
I applied changes as suggested in all the remaining comments.
Committed as r241054.
Regards,
Robert
RE: [PATCH 3/4] Add support to run auto-vectorization tests for multiple effective targets
Hi, > On May 5, 2016, at 8:14 AM, Robert Suchanek > wrote: > > > > I'm resending this patch as it has been rebased and updated. I reverted a > change > > to check_effective_target_vect_call_lrint procedure because it does not use > > cached result. > > Ok. > > Please ensure that the compilation flag is mixed into the test case name so > that as you iterate over them, the test case names are unique. An effective target is likely to have a unique flag to enable a given set of SIMD operations and this is mixed into test case names. I double-checked this with mips-mti-linux-gnu where auto-vectorization tests can be run twice i.e. for -mmsa and -mpaired-single. The patch was rebased once again and tested on x86_64-unknown-linux-gnu. Committed as r238755. Thanks and regards, Robert
RE: [PATCH] MIPS MSA: Fix ICE when using out-of-range values to intrinsics
Hi,
> Robert Suchanek writes:
> > The patch primarily fixes an ICE with out-of-range values to the
> > __builtin_msa_insve* intrinsics.
> >
> > The compiler segfaults in mips_legitimize_const_move () as it tries to
> > split symbol that has NULL_RTX value and gets here because the patterns
> > reject the operand and a new move for the constant is being introduced.
> >
> > The INSVE.DF instruction cannot use register based access to the
> > element, thus, the attempt is obviously wrong and I think that we should
> > be catching this invalid input early.
>
> Agreed, catching problems with arguments to builtins early sounds better
> to me generally.
>
> > I took the opportunity to check all the builtins with literal integers
> > except those that use full range of a type as truncation warnings are
> > generated. The diagnostics is slightly more meaningful and the valid
> > ranges align with the documentation.
>
> Some comments on the implementation below. Just some further cleanup.
>
> > gcc/
> > * config/mips/mips.c (mips_expand_builtin_insn): Check input ranges
> >of literal integer arguments.
> >
> > gcc/testsuite/
> >
> > * gcc.target/mips/msa-builtins-err.c: New test.
> > ---
> > gcc/config/mips/mips.c | 56 +-
> > gcc/testsuite/gcc.target/mips/msa-builtins-err.c | 241
> > +++
> > 2 files changed, 289 insertions(+), 8 deletions(-) create mode 100644
> > gcc/testsuite/gcc.target/mips/msa-builtins-err.c
> >
> > diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> > 44cdeb7..ddb64fb 100644
> > --- a/gcc/config/mips/mips.c
> > +++ b/gcc/config/mips/mips.c
> > @@ -16542,6 +16542,7 @@ mips_expand_builtin_insn (enum insn_code icode,
> > unsigned int nops,
> > struct expand_operand *ops, bool has_target_p) {
> >machine_mode imode;
> > + HOST_WIDE_INT arglo, arghi, argno = -1;
>
> arglo/arghi should really be initialised to zero as the control flow is
> complex that ensures they are set when argno != -1. Perhaps rangelo
> and rangehi to distinguish from the operand numbers.
>
> argno (perhaps error_opno) can be an int.
>
> >
> >switch (icode)
> > {
> > @@ -16570,11 +16571,18 @@ mips_expand_builtin_insn (enum insn_code
> > icode, unsigned int nops,
> > case CODE_FOR_msa_subvi_w:
> > case CODE_FOR_msa_subvi_d:
> >gcc_assert (has_target_p && nops == 3);
> > + arglo = 0;
> > + arghi = 31;
> >/* We only generate a vector of constants iff the second argument
> > is an immediate. We also validate the range of the immediate. */
> > - if (!CONST_INT_P (ops[2].value)
> > - || !IN_RANGE (INTVAL (ops[2].value), 0, 31))
> > + if (!CONST_INT_P (ops[2].value))
> > break;
> > + if (CONST_INT_P (ops[2].value)
> > + && !IN_RANGE (INTVAL (ops[2].value), arglo, arghi))
> > + {
> > + argno = 2;
> > + break;
> > + }
>
> The error should really be shown if a !CONST_INT_P is found too. I.e.
As discussed offline, unfortunately we fake some of the built-ins by creating
aliases and a single pattern may handle a built-in with an immediate or register
operand. I applied suggestions with some modifications. It's still not ideal
but slightly easier to understand the code and flow.
>
> if (!CONST_INT_P (ops[2].value)
> || !IN_RANGE (INTVAL (ops[2].value), 0, 31))
> {
> error_opno = 2;
> break;
> }
>
> Similarly throughout.
>
> >ops[2].mode = ops[0].mode;
> >ops[2].value = mips_gen_const_int_vector (ops[2].mode,
> > INTVAL (ops[2].value));
> > @@ -16601,11 +16609,18 @@ mips_expand_builtin_insn (enum insn_code
> > icode, unsigned int nops,
> > case CODE_FOR_msa_mini_s_w:
> > case CODE_FOR_msa_mini_s_d:
> >gcc_assert (has_target_p && nops == 3);
> > + arglo = -16;
> > + arghi = 15;
> >/* We only generate a vector of constants iff the second argument
> > is an immediate. We also validate the range of the immediate. */
> > - if (!CONST_INT_P (ops[2].value)
> > - || !IN_RANGE (INTVAL (ops[2].value), -16, 15))
> > + if (!CONST_INT_P (ops[2].value))
> > break;
> > + if (CONST_INT_P (ops[2].value)
> > + && !IN_RANGE (INTVAL (ops[2].value), arglo, arghi))
> > + {
> > + argno = 2;
> > + break;
>
RE: [PATCH] MIPS MSA: Fix ICE when using out-of-range values to intrinsics
Hi, Committed as r243301. Regards, Robert > > Robert Suchanek writes: > > The revised patch attached below. > > > > Regards, > > Robert > > > > gcc/ > > * config/mips/mips.c (mips_expand_builtin_insn): Check input ranges > > of literal integer arguments. > > > > gcc/testsuite/ > > > > * gcc.target/mips/msa-builtins-err.c: New test. > > OK, thanks for the rework. > > Matthew
RE: [PATCH 1/4] [MIPS] Add support for MIPS SIMD Architecture (MSA)
Hi Matthew,
Revised patch attached.
Tested with mips-img-linux-gnu and bootstrapped x86_64-unknown-linux-gnu.
> > mips_gen_const_int_vector
> This should use gen_int_for_mode instead of GEN_INT to avoid the issues that
> msa_ldi is
> trying to handle.
gen_int_mode cannot be used to generate a vector of constants as it expects a
scalar mode.
AFAICS, there isn't any generic helper to replace this.
>
> > mips_const_vector_same_bytes_p
> comment on this function is same as previous function
Corrected.
>
> > mips_msa_idiv_insns
> Why not just update mips_idiv_insns and add a mode argument?
Done.
>
> > Implement TARGET_PRINT_OPERAND.
> Comment spacing between 'E' 'B' and description is different to existing
Updated.
>
> > mips_print_operand
> case 'v' subcases V4SImode and V4SFmode are identical. same for DI/DF.
Updated.
>
> >@@ -12272,13 +12837,25 @@ mips_class_max_nregs (enum reg_class rclass,
> machine_mode mode)
> > if (hard_reg_set_intersect_p (left, reg_class_contents[(int) ST_REGS]))
> > {
> > if (HARD_REGNO_MODE_OK (ST_REG_FIRST, mode))
> >-size = MIN (size, 4);
> >+{
> >+ if (MSA_SUPPORTED_MODE_P (mode))
> >+size = MIN (size, UNITS_PER_MSA_REG);
> >+ else
> >+size = MIN (size, UNITS_PER_FPREG);
> >+}
> >+
>
> This hunk should be removed. MSA modes are not supported in ST_REGS.
Indeed. Removed.
>
> >@@ -12299,6 +12876,10 @@ mips_cannot_change_mode_class (machine_mode from,
> > && INTEGRAL_MODE_P (from) && INTEGRAL_MODE_P (to))
> > return false;
> >
> >+ /* Allow conversions between different MSA vector modes and TImode. */
>
> Remove 'and TImode' we do not support it.
Done.
>
> >@@ -19497,9 +21284,64 @@ mips_expand_vec_unpack (rtx operands[2], bool
> unsigned_p, bool high_p)
> >+if (!unsigned_p)
> >+{
> >+ /* Extract sign extention for each element comparing each element with
> >+ immediate zero. */
> >+ tmp = gen_reg_rtx (imode);
> >+ emit_insn (cmpFunc (tmp, operands[1], CONST0_RTX (imode)));
> >+}
> >+else
> >+{
> >+ tmp = force_reg (imode, CONST0_RTX (imode));
> >+}
>
> Indentation and unnecessary braces on the else.
Fixed.
>
> + A single N-word move is usually the same cost as N single-word moves.
> + For MSA, we set MOVE_MAX to 16 bytes.
> + Then, MAX_MOVE_MAX is 16 unconditionally. */
> +#define MOVE_MAX (TARGET_MSA ? 16 : UNITS_PER_WORD)
> +#define MAX_MOVE_MAX 16
>
> The 16 here should be UNITS_PER_MSA_REG
>
The changes have been reverted because of link to MAX_FIXED_MODE_SIZE macro
causing failures in the by_pieces logic if MOVE_MAX_PIECES is larger than
MAX_FIXED_MODE_SIZE.
As it stands, vector modes appear to be handled explicitly in the common code
so it's unlikely we need to modify any of these.
If they do then it will be included in the follow up.
> > mips_expand_builtin_insn
>
> General comment about operations that take an immediate. There is code to
> perform range
> checking but it does not seem to leave any trail when the maybe_expand_insn
> fails to
> tell the user it was an out of range immediate that was the problem. (follow
> up
> work)
Will do.
>
> >+case CODE_FOR_msa_andi_b:
> >+case CODE_FOR_msa_ori_b:
> >+case CODE_FOR_msa_nori_b:
> >+case CODE_FOR_msa_xori_b:
> >+ gcc_assert (has_target_p && nops == 3);
> >+ if (!CONST_INT_P (ops[2].value))
> >+break;
> >+ ops[2].mode = ops[0].mode;
> >+ /* We need to convert the unsigned value to signed. */
> >+ val = sext_hwi (INTVAL (ops[2].value),
> >+ GET_MODE_UNIT_PRECISION (ops[2].mode));
> >+ ops[2].value = mips_gen_const_int_vector (ops[2].mode, val);
> >+ break
>
> Isn't the sext_hwi just effectively doing what gen_int_for_mode would? Fixing
> mips_gen_const_int_vector would eliminate all of them.
That's correct. I've moved it to mips_gen_cost_int_vector and used gen_int_mode.
>
> >@@ -527,7 +551,9 @@ (define_attr "insn_count" ""
> > (const_int 2)
> >
> > (eq_attr "type" "idiv,idiv3")
> >- (symbol_ref "mips_idiv_insns ()")
> >+ (cond [(eq_attr "mode" "TI")
> >+(symbol_ref "mips_msa_idiv_insns () * 4")]
> >+(symbol_ref "mips_idiv_insns () * 4"))
>
> Why *4?
I'm not sure but it appears to be introduced long ago.
I removed it and used only mips_idiv_insns with the mode.
>
> >@@ -1537,8 +1553,10 @@ FP_ASM_SPEC "\
> > #define LONG_LONG_ACCUM_TYPE_SIZE (TARGET_64BIT ? 128 : 64)
> >
> > /* long double is not a fixed mode, but the idea is that, if we
> >- support long double, we also want a 128-bit integer type. */
> >-#define MAX_FIXED_MODE_SIZE LONG_DOUBLE_TYPE_SIZE
> >+ support long double, we also want a 128-bit integer type.
> >+ For MSA, we support an integer type with a width of BITS_PER_MSA_REG. */
> >+#define MAX_FIXED_MODE_SIZE \
> >+ (TARGET_MSA ? BITS_PER_MSA_REG : LONG_DOUBLE_TYPE_SIZE)
>
> This doesn't seem right. We don't support TImode
RE: [PATCH 1/4] [MIPS] Add support for MIPS SIMD Architecture (MSA)
Hi Matthew, > One small tweak, ChangeLog should wrap at 74 columns. Done. > Please consider the > full list of authors for this patch as it has had many major contributors > now. I believe this includes at least the following for the implementation > but fewer for the testsuite updates: > > Robert Suchanek > Sameera Deshpande > Matthew Fortune > Graham Stott > Chao-ying Fu Of course. All patches have or will have the correct contributors in ChangeLog. > > Otherwise, OK to commit! I removed __builtin_msa_[d]lsa from extend.texi as part of the pre-commit checks. These two were listed in the docs but never defined. Removed as obvious. Committed as r236030. Regards, Robert
RE: [PATCH 2/4] [MIPS] Add pipeline description for MSA
Hi Matthew, > > gcc/ChangeLog: > > > > * config/mips/i6400.md (i6400_fpu_intadd, i6400_fpu_logic) > > (i6400_fpu_div, i6400_fpu_cmp, i6400_fpu_float, i6400_fpu_store) > > (i6400_fpu_long_pipe, i6400_fpu_logic_l, i6400_fpu_float_l) > > (i6400_fpu_mult): New cpu units. > > (i6400_msa_add_d, i6400_msa_int_add, i6400_msa_short_logic3) > > (i6400_msa_short_logic2, i6400_msa_short_logic, i6400_msa_move) > > (i6400_msa_cmp, i6400_msa_short_float2, i6400_msa_div_d) > > (i6400_msa_div_w, i6400_msa_div_h, i6400_msa_div_b, i6400_msa_copy) > > (i6400_msa_branch, i6400_fpu_msa_store, i6400_fpu_msa_load) > > (i6400_fpu_msa_move, i6400_msa_long_logic1, i6400_msa_long_logic2) > > (i6400_msa_mult, i6400_msa_long_float2, i6400_msa_long_float4) > > (i6400_msa_long_float5, i6400_msa_long_float8, i6400_msa_fdiv_df) > > (i6400_msa_fdiv_sf): New reservations. > > * config/mips/p5600.md (p5600_fpu_intadd, p5600_fpu_cmp) > > (p5600_fpu_float, p5600_fpu_logic_a, p5600_fpu_logic_b, > > p5600_fpu_div) > > (p5600_fpu_logic, p5600_fpu_float_a, p5600_fpu_float_b,) > > Typo with "," at the end of the list > > > (p5600_fpu_float_c, p5600_fpu_float_d, p5600_fpu_mult, > > p5600_fpu_fdiv) > > (p5600_fpu_load): New cpu units. > > (msa_short_int_add, msa_short_logic, msa_short_logic_move_v) > > (msa_short_cmp, msa_short_float2, msa_short_logic3, > > msa_short_store4) > > (msa_long_load, msa_short_store, msa_long_logic, msa_long_float2) > > (msa_long_float4, msa_long_float5, msa_long_float8, msa_long_mult) > > (msa_long_fdiv, msa_long_div): New reservations. > > I assume this patch has not changed since it was posted. That's correct. > > OK to commit. Committed as r236031. Regards, Robert
[PATCH][MIPS] Enable LSA/DLSA for MSA
Hi,
The below enables LSA/DLSA instructions for -mmsa.
Ok to commit?
Regards,
Robert
* config/mips/mips.h (ISA_HAS_LSA): Enable for -mmsa.
(ISA_HAS_DLSA): Ditto.
---
gcc/config/mips/mips.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 1efa61a..e8897d1 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -209,10 +209,12 @@ struct mips_cpu_info {
#endif
/* ISA has LSA available. */
-#define ISA_HAS_LSA(mips_isa_rev >= 6)
+#define ISA_HAS_LSA(mips_isa_rev >= 6 || ISA_HAS_MSA)
/* ISA has DLSA available. */
-#define ISA_HAS_DLSA (TARGET_64BIT && mips_isa_rev >= 6)
+#define ISA_HAS_DLSA (TARGET_64BIT \
+&& (mips_isa_rev >= 6 \
+|| ISA_HAS_MSA))
/* The ISA compression flags that are currently in effect. */
#define TARGET_COMPRESSION (target_flags & (MASK_MIPS16 | MASK_MICROMIPS))
--
2.8.2.396.g5fe494c
[PATCH][MIPS] Correct latency of loads in M5100
Hi, A small patch to correct the latency for M5100. Ok to commit? Regards, Robert 2016-05-13 Matthew Fortune * config/mips/m5100.md (m51_int_load): Update the latency to 2. --- gcc/config/mips/m5100.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/mips/m5100.md b/gcc/config/mips/m5100.md index f69fc7f..8d87b70 100644 --- a/gcc/config/mips/m5100.md +++ b/gcc/config/mips/m5100.md @@ -65,7 +65,7 @@ (define_insn_reservation "m51_int_jump" 1 ;; loads: lb, lbu, lh, lhu, ll, lw, lwl, lwr, lwpc, lwxs ;; prefetch: prefetch, prefetchx -(define_insn_reservation "m51_int_load" 3 +(define_insn_reservation "m51_int_load" 2 (and (eq_attr "cpu" "m5100") (eq_attr "type" "load,prefetch,prefetchx")) "m51_alu") -- 2.8.2.396.g5fe494c
RE: [PATCH][MIPS] Correct latency of loads in M5100
> > Ok to commit? > > > * config/mips/m5100.md (m51_int_load): Update the latency to 2. > > OK. Committed - r236288 Robert
RE: [PATCH][MIPS] Enable LSA/DLSA for MSA
Hi Matthew, > > Ok to commit? > > OK. Done as r236289. > There is a corresponding testsuite change needed for this > as some code quality tests change if LSA is available. This > is the HAS_LSA 'ghost' option in mips.exp. I'm happy to leave > this to be dealt with as part of the overall MSA testsuite > patch though. It's on my TODO list and will update the patch with MSA tests to put all tests in one go. Regards, Robert
[PATCH][MIPS] Add support for P6600
Hi,
The below patch adds support for MIPS P6600 CPU.
This patch will go in after the approval of the Binutils patch.
Tested with mips-img-linux-gnu.
Regards,
Robert
2016-05-20 Matthew Fortune
Prachi Godbole
* config/mips/mips-cpus.def: Add definition for p6600.
* config/mips/mips-tables.opt: Regenerate.
* config/mips/mips.c (mips_ucbranch_type): New enum.
(mips_rtx_cost_data): Add costs for p6600.
(mips_issue_rate): Add support for p6600.
(mips_multipass_dfa_lookahead): Likewise.
(mips_classify_branch_p6600): New function.
(mips_avoid_hazard): Optimize unconditional compact branch
hazard.
(mips_reorg_process_insns): Likewise.
* config/mips/mips.h (TUNE_P6600): New define.
(MIPS_ISA_LEVEL_SPEC): Infer mips64r6 from p6600.
(ENABLE_LD_ST_PAIRS): Enable load/store pairs for p6600.
* config/mips/mips.md: Include p6600.md.
(processor): Add p6600.
(JOIN_MODE): Add support for load/store pairs for 64-bit target.
* config/mips/p6600.md: New file.
* doc/invoke.texi: Add p6600 to supported architectures.
---
gcc/config/mips/mips-cpus.def | 1 +
gcc/config/mips/mips-tables.opt | 3 +
gcc/config/mips/mips.c | 84 +-
gcc/config/mips/mips.h | 6 +-
gcc/config/mips/mips.md | 3 +
gcc/config/mips/p6600.md| 349
gcc/doc/invoke.texi | 2 +-
7 files changed, 442 insertions(+), 6 deletions(-)
create mode 100644 gcc/config/mips/p6600.md
diff --git a/gcc/config/mips/mips-cpus.def b/gcc/config/mips/mips-cpus.def
index 5df9807..5694e87 100644
--- a/gcc/config/mips/mips-cpus.def
+++ b/gcc/config/mips/mips-cpus.def
@@ -171,3 +171,4 @@ MIPS_CPU ("xlp", PROCESSOR_XLP, 65, PTF_AVOID_BRANCHLIKELY)
/* MIPS64 Release 6 processors. */
MIPS_CPU ("i6400", PROCESSOR_I6400, 69, 0)
+MIPS_CPU ("p6600", PROCESSOR_P6600, 69, 0)
diff --git a/gcc/config/mips/mips-tables.opt b/gcc/config/mips/mips-tables.opt
index 34c12bd..270fcc0 100644
--- a/gcc/config/mips/mips-tables.opt
+++ b/gcc/config/mips/mips-tables.opt
@@ -696,3 +696,6 @@ Enum(mips_arch_opt_value) String(xlp) Value(101) Canonical
EnumValue
Enum(mips_arch_opt_value) String(i6400) Value(102) Canonical
+EnumValue
+Enum(mips_arch_opt_value) String(p6600) Value(103) Canonical
+
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 06acd30..cbe1007 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -194,6 +194,15 @@ enum mips_address_type {
ADDRESS_SYMBOLIC
};
+/* Classifies an unconditional branch of interest for the P6600. */
+
+enum mips_ucbranch_type {
+ /* May not even be a branch. */
+ UC_UNDEFINED,
+ UC_BALC,
+ UC_OTHER
+};
+
/* Macros to create an enumeration identifier for a function prototype. */
#define MIPS_FTYPE_NAME1(A, B) MIPS_##A##_FTYPE_##B
#define MIPS_FTYPE_NAME2(A, B, C) MIPS_##A##_FTYPE_##B##_##C
@@ -1122,6 +1131,19 @@ static const struct mips_rtx_cost_data
COSTS_N_INSNS (36), /* int_div_di */
2,/* branch_cost */
4 /* memory_latency */
+ },
+ { /* P6600 */
+COSTS_N_INSNS (4),/* fp_add */
+COSTS_N_INSNS (5),/* fp_mult_sf */
+COSTS_N_INSNS (5),/* fp_mult_df */
+COSTS_N_INSNS (17), /* fp_div_sf */
+COSTS_N_INSNS (17), /* fp_div_df */
+COSTS_N_INSNS (5),/* int_mult_si */
+COSTS_N_INSNS (5),/* int_mult_di */
+COSTS_N_INSNS (8),/* int_div_si */
+COSTS_N_INSNS (8),/* int_div_di */
+ 2,/* branch_cost */
+ 4 /* memory_latency */
}
};
@@ -14507,6 +14529,7 @@ mips_issue_rate (void)
case PROCESSOR_LOONGSON_2F:
case PROCESSOR_LOONGSON_3A:
case PROCESSOR_P5600:
+case PROCESSOR_P6600:
return 4;
case PROCESSOR_XLP:
@@ -14642,7 +14665,7 @@ mips_multipass_dfa_lookahead (void)
if (TUNE_OCTEON)
return 2;
- if (TUNE_P5600 || TUNE_I6400)
+ if (TUNE_P5600 || TUNE_I6400 || TUNE_P6600)
return 4;
return 0;
@@ -18496,6 +18519,28 @@ mips_orphaned_high_part_p (mips_offset_table *htab,
rtx_insn *insn)
return false;
}
+/* Subroutine of mips_avoid_hazard. We classify unconditional branches
+ of interest for the P6600 for performance reasons. We are interested
+ in differentiating BALC from JIC, JIALC and BC. */
+
+static enum mips_ucbranch_type
+mips_classify_branch_p6600 (rtx_insn *insn)
+{
+ if (!(insn
+ && USEFUL_INSN_P (insn)
+ && GET_CODE (PATTERN (insn)) != SEQUENCE))
+return UC_UNDEFINED;
+
+ if (get_attr_jal (insn) == JAL_INDIRECT /* JIC and JIALC. */
+ || get_attr_type (insn) == TYPE_JUMP) /* BC as it is a loose jump. */
+return UC_OTHER;
+
+ if (CALL_P (insn) && get_attr_jal (insn) == JAL_DI
[PATCH][MIPS] Fix ICE for constant pool data in GP area for MIPS16
Hi,
The patch fixes an ICE when the compiler tries to split an instruction.
Test attached.
No regression. Ok to apply?
Regards,
Robert
gcc/
2016-05-20 Andrew Bennett
* config/mips/mips.c (mips_constant_pool_symbol_in_sdata_p): New
function.
(mips_output_move): Copy GP instead of splitting HIGH when
accessing constant pool data.
gcc/testsuite/
2016-05-20 Robert Suchanek
* gcc.target/mips/mips16-gp-bug-1.c: New test.
---
gcc/config/mips/mips.c | 20 +++-
gcc/testsuite/gcc.target/mips/mips16-gp-bug-1.c | 12
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/mips/mips16-gp-bug-1.c
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index cbe1007..e6d8f76 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -5053,6 +5053,18 @@ mips_split_move_insn (rtx dest, rtx src, rtx insn)
mips_split_move (dest, src, mips_insn_split_type (insn));
}
+/* Return true if X is a GP-relative symbol in the constant pool.
+ CONTEXT is the context in which X appears. */
+
+bool
+mips_constant_pool_symbol_in_sdata_p (rtx x, enum mips_symbol_context context)
+{
+ enum mips_symbol_type symbol_type;
+ return (mips_symbolic_constant_p (x, context, &symbol_type)
+ && symbol_type == SYMBOL_GP_RELATIVE
+ && CONSTANT_POOL_ADDRESS_P (x));
+}
+
/* Return the appropriate instructions to move SRC into DEST. Assume
that SRC is operand 1 and DEST is operand 0. */
@@ -5202,7 +5214,13 @@ mips_output_move (rtx dest, rtx src)
}
if (src_code == HIGH)
- return TARGET_MIPS16 ? "#" : "lui\t%0,%h1";
+ {
+ if (mips_constant_pool_symbol_in_sdata_p (XEXP (src, 0),
+ SYMBOL_CONTEXT_MEM))
+ return "move\t%0,$28";
+
+ return TARGET_MIPS16 ? "#" : "lui\t%0,%h1";
+ }
if (CONST_GP_P (src))
return "move\t%0,%1";
diff --git a/gcc/testsuite/gcc.target/mips/mips16-gp-bug-1.c
b/gcc/testsuite/gcc.target/mips/mips16-gp-bug-1.c
new file mode 100644
index 000..85a6d83
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/mips16-gp-bug-1.c
@@ -0,0 +1,12 @@
+/* { dg-options "(-mips16) -G4 -mno-abicalls -mcode-readable=no" } */
+typedef struct { int a, b, c, d; } A[5000];
+int a, b;
+extern void bar (int, int, A);
+
+void
+foo (int p1)
+{
+ A c;
+ while (p1)
+bar (a, b, c);
+}
--
2.8.2.396.g5fe494c
[PATCH][MIPS] Add -mgrow-frame-downwards option
Hi,
The patch changes the default behaviour of the direction in which
the local frame grows for MIPS16.
The code size reduces by about 0.5% in average case for -Os, hence,
it is good to turn the option on by default.
Ok to apply?
Regards,
Robert
gcc/
2016-05-20 Matthew Fortune
* config/mips/mips.h (FRAME_GROWS_DOWNWARD): Enable it
conditionally for MIPS16.
* config/mips/mips.opt: Add -mgrow-frame-downwards option.
Enable it by default for MIPS16.
* doc/invoke.texi: Document the option.
---
gcc/config/mips/mips.h | 8 +++-
gcc/config/mips/mips.opt | 4
gcc/doc/invoke.texi | 13 +
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 5020208..6ab7dd3 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -2311,7 +2311,13 @@ enum reg_class
#define STACK_GROWS_DOWNWARD 1
-#define FRAME_GROWS_DOWNWARD flag_stack_protect
+/* Growing the frame downwards allows us to put spills closest to
+ the stack pointer which is good as they are likely to be accessed
+ frequently. We can also arrange for normal stack usage to place
+ scalars last so that they too are close to the stack pointer. */
+#define FRAME_GROWS_DOWNWARD ((TARGET_MIPS16 \
+ && TARGET_FRAME_GROWS_DOWNWARDS) \
+ || flag_stack_protect)
/* Size of the area allocated in the frame to save the GP. */
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index 3b92ef5..53feb23 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -447,3 +447,7 @@ Enum(mips_cb_setting) String(always) Value(MIPS_CB_ALWAYS)
minline-intermix
Target Report Var(TARGET_INLINE_INTERMIX)
Allow inlining even if the compression flags differ between caller and callee.
+
+mgrow-frame-downwards
+Target Report Var(TARGET_FRAME_GROWS_DOWNWARDS) Init(1)
+Change the behaviour to grow the frame downwards for MIPS16.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 2f6195e..6e5d620 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -838,6 +838,7 @@ Objective-C and Objective-C++ Dialects}.
-minterlink-compressed -mno-interlink-compressed @gol
-minterlink-mips16 -mno-interlink-mips16 @gol
-minline-intermix -mno-inline-intermix @gol
+-mgrow-frame-downwards -mno-grow-frame-downwards @gol
-mabi=@var{abi} -mabicalls -mno-abicalls @gol
-mshared -mno-shared -mplt -mno-plt -mxgot -mno-xgot @gol
-mgp32 -mgp64 -mfp32 -mfpxx -mfp64 -mhard-float -msoft-float @gol
@@ -17929,6 +17930,18 @@ vice-versa. When using this option it is necessary to
protect functions
that cannot be compiled as MIPS16 with a @code{noinline} attribute to ensure
they are not inlined into a MIPS16 function.
+@item -mgrow-frame-downwards
+@itemx -mno-grow-frame-downwards
+@opindex mgrow-frame-downwards
+Grow the local frame down (up) for MIPS16.
+
+Growing the frame downwards allows us to get spill slots created at the lowest
+address rather than the highest address in a local frame. The benefit of this
+is smaller code size as accessing spill splots closer to the stack pointer
+can be done using using 16-bit instructions.
+
+The option is enabled by default (to grow frame downwards) for MIPS16.
+
@item -mabi=32
@itemx -mabi=o64
@itemx -mabi=n32
--
2.8.2.396.g5fe494c
[PATCH][MIPS] Disable madd/msub when -mno-imadd is used with -mdsp
Hi,
If -mdsp option is used then adding -mno-imadd has no effect on the code
generation. This appears to be slightly inconsistent to the -m[no-]imadd option
we have.
Any potential problems/comments? Ok to commit?
Regards,
Robert
gcc/
* config/mips/mips.c (mips_option_override): Move DSP ASE checks
earlier in the function. Enable madd/msub patterns by default
for DSP ASE.
* config/mips/mips.md (msubsidi4): Remove override for DSP.
(maddsidi4): Ditto.
---
gcc/config/mips/mips.c | 30 --
gcc/config/mips/mips.md | 4 ++--
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 4312368..9c6c2a5 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19758,13 +19758,27 @@ mips_option_override (void)
warning (0, "the %qs architecture does not support branch-likely"
" instructions", mips_arch_info->name);
+ /* If TARGET_DSPR2, enable TARGET_DSP. */
+ if (TARGET_DSPR2)
+TARGET_DSP = true;
+
+ if (TARGET_DSP && mips_isa_rev >= 6)
+{
+ error ("the %qs architecture does not support DSP instructions",
+mips_arch_info->name);
+ TARGET_DSP = false;
+ TARGET_DSPR2 = false;
+}
+
/* If the user hasn't specified -mimadd or -mno-imadd set
MASK_IMADD based on the target architecture and tuning
flags. */
if ((target_flags_explicit & MASK_IMADD) == 0)
{
- if (ISA_HAS_MADD_MSUB &&
- (mips_tune_info->tune_flags & PTF_AVOID_IMADD) == 0)
+ if ((ISA_HAS_MADD_MSUB &&
+ (mips_tune_info->tune_flags & PTF_AVOID_IMADD) == 0)
+ /* Enable for DSP by default */
+ || TARGET_DSP)
target_flags |= MASK_IMADD;
else
target_flags &= ~MASK_IMADD;
@@ -19955,18 +19969,6 @@ mips_option_override (void)
mips_r10k_cache_barrier = R10K_CACHE_BARRIER_NONE;
}
- /* If TARGET_DSPR2, enable TARGET_DSP. */
- if (TARGET_DSPR2)
-TARGET_DSP = true;
-
- if (TARGET_DSP && mips_isa_rev >= 6)
-{
- error ("the %qs architecture does not support DSP instructions",
-mips_arch_info->name);
- TARGET_DSP = false;
- TARGET_DSPR2 = false;
-}
-
/* .eh_frame addresses should be the same width as a C pointer.
Most MIPS ABIs support only one pointer size, so the assembler
will usually know exactly how big an .eh_frame address is.
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 432ab1a..22f4f0b 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -2242,7 +2242,7 @@ (define_insn "msubsidi4"
(mult:DI
(any_extend:DI (match_operand:SI 1 "register_operand" "d"))
(any_extend:DI (match_operand:SI 2 "register_operand" "d")]
- "!TARGET_64BIT && (ISA_HAS_MSAC || GENERATE_MADD_MSUB || ISA_HAS_DSP)"
+ "!TARGET_64BIT && (ISA_HAS_MSAC || GENERATE_MADD_MSUB)"
{
if (ISA_HAS_DSP_MULT)
return "msub\t%q0,%1,%2";
@@ -2516,7 +2516,7 @@ (define_insn "maddsidi4"
(mult:DI (any_extend:DI (match_operand:SI 1 "register_operand" "d"))
(any_extend:DI (match_operand:SI 2 "register_operand" "d")))
(match_operand:DI 3 "muldiv_target_operand" "0")))]
- "(TARGET_MAD || ISA_HAS_MACC || GENERATE_MADD_MSUB || ISA_HAS_DSP)
+ "(TARGET_MAD || ISA_HAS_MACC || GENERATE_MADD_MSUB)
&& !TARGET_64BIT"
{
if (TARGET_MAD)
--
2.8.2.396.g5fe494c
RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture
Hi Catherine,
Apologies for the (very) late reply.
It appears that I never replied to the last message.
> > gcc/
> > * config/mips/mips-cpus.def: Replace PTF_AVOID_BRANCHLIKELY
> > with
> > PTF_AVOID_BRANCHLIKELY_ALWAYS for generic architecture and
> > with
> > PTF_AVOID_BRANCHLIKELY_SPEED for others.
> > (mips2, mips3, mips4): Add PTF_AVOID_BRANCHLIKELY_SIZE to tune
> > flags.
> > * config/mips/mips.c (mips_option_override): Enable the branch
> > likely
> > depending on the tune flags and optimization level.
> > * config/mips/mips.h (PTF_AVOID_BRANCHLIKELY): Remove.
> > (PTF_AVOID_BRANCHLIKELY_SPEED): Define.
> > (PTF_AVOID_BRANCHLIKELY_SIZE): Likewise.
> > (PTF_AVOID_BRANCHLIKELY_ALWAYS): Likewise.
> > ---
> > gcc/config/mips/mips-cpus.def | 56 +---
> > ---
> > gcc/config/mips/mips.c| 6 +++--
> > gcc/config/mips/mips.h| 20
> > 3 files changed, 47 insertions(+), 35 deletions(-)
> >
> > a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 0e0ecf2..f8775c4
> > 100644
> > --- a/gcc/config/mips/mips.c
> > +++ b/gcc/config/mips/mips.c
> > @@ -17916,8 +17916,10 @@ mips_option_override (void)
> >if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
> > {
> >if (ISA_HAS_BRANCHLIKELY
> > - && (optimize_size
> > - || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY)
> > == 0))
> > + && ((optimize_size && (mips_tune_info->tune_flags
> > +& PTF_AVOID_BRANCHLIKELY_SIZE) == 0)
> > + || (!optimize_size && (mips_tune_info->tune_flags
> > + & PTF_AVOID_BRANCHLIKELY_SPEED) ==
> > 0)))
> > target_flags |= MASK_BRANCHLIKELY;
> >else
> > target_flags &= ~MASK_BRANCHLIKELY;
>
> Should this check be:
> Index: mips.c
> ===
> --- mips.c (revision 229138)
> +++ mips.c (working copy)
> @@ -17758,8 +17758,15 @@
>if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
> {
>if (ISA_HAS_BRANCHLIKELY
> - && (optimize_size
> - || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0))
> + && ((optimize_size
> + && (mips_tune_info->tune_flags
> + & PTF_AVOID_BRANCHLIKELY_SIZE) == 0)
> + || (!optimize_size
> +&& optimize > 0
> +&& ((mips_tune_info->tune_flags
> + & PTF_AVOID_BRANCHLIKELY_SPEED) == 0))
> +|| (mips_tune_info->tune_flags
> + & PTF_AVOID_BRANCHLIKELY_ALWAYS) == 0))
> target_flags |= MASK_BRANCHLIKELY;
>else
> target_flags &= ~MASK_BRANCHLIKELY;
>
> Instead? I don't see a use of PTF_AVOID_BRANCH_ALWAYS in your patch, but it
> seems like it should be checked.
>
I did that on purpose at the time as the check looked redundant as it will be
one
or the other. However, for easier reading and a potential redefinition of
*_ALWAYS
e.g. to a unique value then the extra check is a must.
I'm happy to include this. Ok to commit with this change?
Regards,
Robert
[PATCH][MIPS] Add -minline-intermix to ignore compression flags when inlining
Hi,
The below allows us to inline functions that have different compression flags
for better tuning of performance/code size balance.
Ok to commit?
Regards,
Robert
2016-05-24 Matthew Fortune
gcc/
* config/mips/mips.c (mips_can_inline_p): Allow inlining of
functions with opposing compression flags.
* config/mips/mips.opt (minline-intermix): New option.
* doc/invoke.texi: Document the new option.
---
gcc/config/mips/mips.c | 3 ++-
gcc/config/mips/mips.opt | 4
gcc/doc/invoke.texi | 13 +
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 5ecde46..4312368 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -1476,7 +1476,8 @@ mips_merge_decl_attributes (tree olddecl, tree newdecl)
static bool
mips_can_inline_p (tree caller, tree callee)
{
- if (mips_get_compress_mode (callee) != mips_get_compress_mode (caller))
+ if (mips_get_compress_mode (callee) != mips_get_compress_mode (caller)
+ && !TARGET_INLINE_INTERMIX)
return false;
return default_target_can_inline_p (caller, callee);
}
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index 08dd83e..3b92ef5 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -443,3 +443,7 @@ Enum(mips_cb_setting) String(optimal) Value(MIPS_CB_OPTIMAL)
EnumValue
Enum(mips_cb_setting) String(always) Value(MIPS_CB_ALWAYS)
+
+minline-intermix
+Target Report Var(TARGET_INLINE_INTERMIX)
+Allow inlining even if the compression flags differ between caller and callee.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 73f1cb6..2f6195e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -837,6 +837,7 @@ Objective-C and Objective-C++ Dialects}.
-mips16 -mno-mips16 -mflip-mips16 @gol
-minterlink-compressed -mno-interlink-compressed @gol
-minterlink-mips16 -mno-interlink-mips16 @gol
+-minline-intermix -mno-inline-intermix @gol
-mabi=@var{abi} -mabicalls -mno-abicalls @gol
-mshared -mno-shared -mplt -mno-plt -mxgot -mno-xgot @gol
-mgp32 -mgp64 -mfp32 -mfpxx -mfp64 -mhard-float -msoft-float @gol
@@ -17916,6 +17917,18 @@ Aliases of @option{-minterlink-compressed} and
@option{-mno-interlink-compressed}. These options predate the microMIPS ASE
and are retained for backwards compatibility.
+@item -minline-intermix
+@itemx -mno-inline-intermix
+@opindex minline-intermix
+@opindex mno-inline-intermix
+Enable inlining of functions which have opposing compression flags e.g.
+@code{mips16}/@code{nomips16} attributes.
+This is useful when using the @code{mips16} attribute to balance code size
+and performance so that a function will be compressed when not inlined or
+vice-versa. When using this option it is necessary to protect functions
+that cannot be compiled as MIPS16 with a @code{noinline} attribute to ensure
+they are not inlined into a MIPS16 function.
+
@item -mabi=32
@itemx -mabi=o64
@itemx -mabi=n32
--
2.8.2.396.g5fe494c
[PATCH][MIPS] Add support for code_readable function attribute
Hi,
The patch adds support for __attribute__ ((code_readable)) with
optional argument that accepts `no', `yes' or `pcrel' just
like the -mcode-readable= command line switch. If the argument
is not specified then the default `yes' is applied.
This of course has only effect on targets supporting -mcode-readable=.
Regards,
Robert
2016-05-24 Matthew Fortune
Simon Dardis
gcc/
* config/mips/mips.c (mips_base_code_readable): New.
(mips_handle_code_readable_attr): New static function.
(mips_get_code_readable_attr): Likewise.
(mips_set_current_function): Add support for the code_readable
attribute.
(mips_option_override): Likewise.
* doc/extend.text: Document the new attribute.
gcc/testsuite/
* gcc.target/mips/code-readable-attr-1.c: New test.
* gcc.target/mips/code-readable-attr-2.c: Ditto.
* gcc.target/mips/code-readable-attr-3.c: Ditto.
* gcc.target/mips/code-readable-attr-4.c: Ditto.
* gcc.target/mips/code-readable-attr-5.c: Ditto.
---
gcc/config/mips/mips.c | 94 +-
gcc/doc/extend.texi| 17
.../gcc.target/mips/code-readable-attr-1.c | 51
.../gcc.target/mips/code-readable-attr-2.c | 49 +++
.../gcc.target/mips/code-readable-attr-3.c | 50
.../gcc.target/mips/code-readable-attr-4.c | 51
.../gcc.target/mips/code-readable-attr-5.c | 5 ++
7 files changed, 316 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/mips/code-readable-attr-1.c
create mode 100644 gcc/testsuite/gcc.target/mips/code-readable-attr-2.c
create mode 100644 gcc/testsuite/gcc.target/mips/code-readable-attr-3.c
create mode 100644 gcc/testsuite/gcc.target/mips/code-readable-attr-4.c
create mode 100644 gcc/testsuite/gcc.target/mips/code-readable-attr-5.c
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 24d98fe..6a469a2 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -492,6 +492,9 @@ static int mips_base_target_flags;
/* The default compression mode. */
unsigned int mips_base_compression_flags;
+/* The default code readable setting. */
+enum mips_code_readable_setting mips_base_code_readable;
+
/* The ambient values of other global variables. */
static int mips_base_schedule_insns; /* flag_schedule_insns */
static int mips_base_reorder_blocks_and_partition; /* flag_reorder... */
@@ -596,6 +599,7 @@ const enum reg_class
mips_regno_to_class[FIRST_PSEUDO_REGISTER] = {
ALL_REGS,ALL_REGS, ALL_REGS, ALL_REGS
};
+static tree mips_handle_code_readable_attr (tree *, tree, tree, int, bool *);
static tree mips_handle_interrupt_attr (tree *, tree, tree, int, bool *);
static tree mips_handle_use_shadow_register_set_attr (tree *, tree, tree, int,
bool *);
@@ -616,6 +620,8 @@ static const struct attribute_spec mips_attribute_table[] =
{
{ "micromips", 0, 0, true, false, false, NULL, false },
{ "nomicromips", 0, 0, true, false, false, NULL, false },
{ "nocompression", 0, 0, true, false, false, NULL, false },
+ { "code_readable", 0, 1, true, false, false, mips_handle_code_readable_attr,
+false },
/* Allow functions to be specified as interrupt handlers */
{ "interrupt", 0, 1, false, true, true, mips_handle_interrupt_attr,
false },
@@ -1296,6 +1302,77 @@ mips_use_debug_exception_return_p (tree type)
TYPE_ATTRIBUTES (type)) != NULL;
}
+/* Verify the arguments to a code_readable attribute. */
+
+static tree
+mips_handle_code_readable_attr (tree *node, tree name, tree args,
+ int flags ATTRIBUTE_UNUSED, bool *no_add_attrs)
+{
+ const char * str;
+
+ if (!is_attribute_p ("code_readable", name) || args == NULL)
+return NULL_TREE;
+
+ if (TREE_CODE (TREE_VALUE (args)) != STRING_CST)
+{
+ warning (OPT_Wattributes,
+ "%qE attribute requires a string argument", name);
+ *no_add_attrs = true;
+}
+ else if (strcmp (TREE_STRING_POINTER (TREE_VALUE (args)), "no") != 0
+ && strcmp (TREE_STRING_POINTER (TREE_VALUE (args)), "pcrel") != 0
+ && strcmp (TREE_STRING_POINTER (TREE_VALUE (args)), "yes") != 0)
+{
+ warning (OPT_Wattributes,
+ "argument to %qE attribute is neither no, pcrel nor yes", name);
+ *no_add_attrs = true;
+}
+
+ return NULL_TREE;
+}
+
+/* Return the code_readable setting for a function if it has one. If there
+ is no argument or the function does not have the attribute, return GCC's
+ default. */
+
+static enum mips_code_readable_setting
+mips_get_code_readable_attr (tree decl)
+{
+ tree attr;
+
+ if (decl == NULL)
+return mips_base_code_readable;
+
+ attr = lookup_attribute ("code_readable", DECL_ATTRIBUTES (decl));
+
+ if (att
[PATCH][MIPS] Don't split shifts by default for MIPS16.
Hi, The following changes the default behaviour of shift splitting for MIPS16 e.g. the shifts will be split only when used with undocumented -mno-debugd option that is now switched on by default. This appears to enable better optimization in certain cases, and hence, giving slightly better performance. Ok to apply? Regards, Robert gcc/ * config/mips/mips.md (3): Don't split shifts when used with -mdebugd. * config/mips/mips.opt (mdebugd): Init to 1 by default. --- gcc/config/mips/mips.md | 1 + gcc/config/mips/mips.opt | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index 22f4f0b..01d7edd 100644 --- a/gcc/config/mips/mips.md +++ b/gcc/config/mips/mips.md @@ -5602,6 +5602,7 @@ (define_expand "3" be careful not to allocate a new register if we've reached the reload pass. */ if (TARGET_MIPS16 + && !TARGET_DEBUG_D_MODE && optimize && CONST_INT_P (operands[2]) && INTVAL (operands[2]) > 8 diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt index 53feb23..b6c839d 100644 --- a/gcc/config/mips/mips.opt +++ b/gcc/config/mips/mips.opt @@ -127,7 +127,7 @@ mdebug Target Var(TARGET_DEBUG_MODE) Undocumented mdebugd -Target Var(TARGET_DEBUG_D_MODE) Undocumented +Target Var(TARGET_DEBUG_D_MODE) Undocumented Init(1) meb Target Report RejectNegative Mask(BIG_ENDIAN) -- 2.8.2.396.g5fe494
[PATCH][MIPS] Remove "new" MIPS TLS access patterns
Hi,
The below finishes the revert of r137670 that was already partially reverted
in r137734 as part of PR target/35802.
It would appear that the revert was not completed because of a spill failure
at the time. As LRA can handle the 'v' constraint just fine and MIPS is going
to drop the support for the classic reload, there is no need for this workaround
that introduces spurious moves hurting the performance.
No regression. Ok to commit?
Regards,
Robert
2016-05-24 Simon Dardis
gcc/
* config/mips/constraints.md (V1_REG): Update comment.
* config/mips/mips.md (get_tls_get_tp_): Remove.
(*get_tls_tp_): Rename.
* doc/md.texi: Update the MIPS "v" constraint.
---
gcc/config/mips/constraints.md | 6 +++---
gcc/config/mips/mips.md| 26 +++---
gcc/doc/md.texi| 3 +--
3 files changed, 7 insertions(+), 28 deletions(-)
diff --git a/gcc/config/mips/constraints.md b/gcc/config/mips/constraints.md
index 56b363e..155b212 100644
--- a/gcc/config/mips/constraints.md
+++ b/gcc/config/mips/constraints.md
@@ -60,11 +60,11 @@ (define_register_constraint "e" "LEA_REGS"
(define_register_constraint "j" "PIC_FN_ADDR_REG"
"@internal")
+;; FIXME: Remove this comment and below once the MIPS backend can
+;; only be used with LRA.
;; Don't use this constraint in gcc code! It runs the risk of
;; introducing a spill failure; see tls_get_tp_.
-(define_register_constraint "v" "V1_REG"
- "Register @code{$3}. Do not use this constraint in new code;
- it is retained only for compatibility with glibc.")
+(define_register_constraint "v" "V1_REG" "@internal")
(define_register_constraint "y" "GR_REGS"
"Equivalent to @code{r}; retained for backwards compatibility.")
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 527f2e1..432ab1a 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -7386,29 +7386,9 @@ (define_insn "*mips16e_save_restore"
;; MIPS 32r2 specification, but we use it on any architecture because
;; we expect it to be emulated. Use .set to force the assembler to
;; accept it.
-;;
-;; We do not use a constraint to force the destination to be $3
-;; because $3 can appear explicitly as a function return value.
-;; If we leave the use of $3 implicit in the constraints until
-;; reload, we may end up making a $3 return value live across
-;; the instruction, leading to a spill failure when reloading it.
-(define_insn_and_split "tls_get_tp_"
- [(set (match_operand:P 0 "register_operand" "=d")
- (unspec:P [(const_int 0)] UNSPEC_TLS_GET_TP))
- (clobber (reg:P TLS_GET_TP_REGNUM))]
- "HAVE_AS_TLS && !TARGET_MIPS16"
- "#"
- "&& reload_completed"
- [(set (reg:P TLS_GET_TP_REGNUM)
- (unspec:P [(const_int 0)] UNSPEC_TLS_GET_TP))
- (set (match_dup 0) (reg:P TLS_GET_TP_REGNUM))]
- ""
- [(set_attr "type" "unknown")
- (set_attr "mode" "")
- (set_attr "insn_count" "2")])
-(define_insn "*tls_get_tp__split"
- [(set (reg:P TLS_GET_TP_REGNUM)
+(define_insn "tls_get_tp_"
+ [(set (match_operand:P 0 "register_operand" "=v")
(unspec:P [(const_int 0)] UNSPEC_TLS_GET_TP))]
"HAVE_AS_TLS && !TARGET_MIPS16"
{
@@ -7418,7 +7398,7 @@ (define_insn "*tls_get_tp__split"
return ".set\tpush\;.set\tmips32r2\t\;rdhwr\t$3,$29\;.set\tpop";
}
[(set_attr "type" "unknown")
- ; Since rdhwr always generates a trap for now, putting it in a delay
+ ; Since rdhwr may generate a trap, putting it in a delay
; slot would make the kernel's emulation of it much slower.
(set_attr "can_delay" "no")
(set_attr "mode" "")])
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index f2360c8..d1c88d2 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -2705,8 +2705,7 @@ A register suitable for use in an indirect jump. This
will always be
@code{$25} for @option{-mabicalls}.
@item v
-Register @code{$3}. Do not use this constraint in new code;
-it is retained only for compatibility with glibc.
+Register @code{$3}. The register for acquiring the TLS pointer.
@item y
Equivalent to @code{r}; retained for backwards compatibility.
--
2.8.2.396.g5fe494
[PATCH][MIPS] P5600 scheduler fix
Hi, The below is a fix for the P5600 scheduler. Ok to commit? Regards, Robert 2016-05-24 Simon Dardis Prachi Godbole gcc/ * config/mips/p5600.md (p5600_fpu_fadd): Remove checking for `fabs' and `fneg' type attributes. (p5600_fpu_fabs): Add `fmove' to the comment. --- gcc/config/mips/p5600.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/config/mips/p5600.md b/gcc/config/mips/p5600.md index 694a745..4500ceb 100644 --- a/gcc/config/mips/p5600.md +++ b/gcc/config/mips/p5600.md @@ -163,10 +163,10 @@ (define_insn_reservation "msa_long_div" 10 ;; fadd, fsub (define_insn_reservation "p5600_fpu_fadd" 4 (and (eq_attr "cpu" "p5600") - (eq_attr "type" "fadd,fabs,fneg")) + (eq_attr "type" "fadd")) "p5600_fpu_long, p5600_fpu_apu") -;; fabs, fneg, fcmp +;; fabs, fneg, fcmp, fmove (define_insn_reservation "p5600_fpu_fabs" 2 (and (eq_attr "cpu" "p5600") (eq_attr "type" "fabs,fneg,fcmp,fmove")) -- 2.8.2.396.g5fe494c
[PATCH 2/4] [MIPS] Add pipeline description for MSA
Hi, The patch adds a pipeline description for MSA to I6400 and P5600 schedulers. Regards, Robert gcc/ChangeLog: * config/mips/i6400.md (i6400_fpu_intadd, i6400_fpu_logic) (i6400_fpu_div, i6400_fpu_cmp, i6400_fpu_float, i6400_fpu_store) (i6400_fpu_long_pipe, i6400_fpu_logic_l, i6400_fpu_float_l) (i6400_fpu_mult): New cpu units. (i6400_msa_add_d, i6400_msa_int_add, i6400_msa_short_logic3) (i6400_msa_short_logic2, i6400_msa_short_logic, i6400_msa_move) (i6400_msa_cmp, i6400_msa_short_float2, i6400_msa_div_d) (i6400_msa_div_w, i6400_msa_div_h, i6400_msa_div_b, i6400_msa_copy) (i6400_msa_branch, i6400_fpu_msa_store, i6400_fpu_msa_load) (i6400_fpu_msa_move, i6400_msa_long_logic1, i6400_msa_long_logic2) (i6400_msa_mult, i6400_msa_long_float2, i6400_msa_long_float4) (i6400_msa_long_float5, i6400_msa_long_float8, i6400_msa_fdiv_df) (i6400_msa_fdiv_sf): New reservations. * config/mips/p5600.md (p5600_fpu_intadd, p5600_fpu_cmp) (p5600_fpu_float, p5600_fpu_logic_a, p5600_fpu_logic_b, p5600_fpu_div) (p5600_fpu_logic, p5600_fpu_float_a, p5600_fpu_float_b,) (p5600_fpu_float_c, p5600_fpu_float_d, p5600_fpu_mult, p5600_fpu_fdiv) (p5600_fpu_load): New cpu units. (msa_short_int_add, msa_short_logic, msa_short_logic_move_v) (msa_short_cmp, msa_short_float2, msa_short_logic3, msa_short_store4) (msa_long_load, msa_short_store, msa_long_logic, msa_long_float2) (msa_long_float4, msa_long_float5, msa_long_float8, msa_long_mult) (msa_long_fdiv, msa_long_div): New reservations. 0002-MIPS-Add-pipeline-description-for-MSA.patch Description: 0002-MIPS-Add-pipeline-description-for-MSA.patch
[PATCH 1/4] [MIPS] Add support for MIPS SIMD Architecture (MSA)
Hi, This series of patches adds the support for MIPS SIMD Architecture (MSA) and underwent a few updates since the last review to address the comments: https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01777.html The series is split into four parts: 0001 [MIPS] Add support for MIPS SIMD Architecture (MSA) 0002 [MIPS] Add pipeline description for MSA 0003 Add support to run auto-vectorization tests for multiple effective targets 0004 [MIPS] Add tests for MSA There a couple things to mention here: - there is a minor regression on O32 ABI due to the lack of stack realignment AFAICS and a patch will follow. The vectorizer generates more unaligned accesses than the tests expect, and hence, fail the checks. - the series doesn't add cost modelling for auto-vectorization - patch 0003 is independent but must go in before 0004. Regards, Robert gcc/ChangeLog: * config.gcc: Add MSA header file for mips*-*-* target. * config/mips/constraints.md (YI, YC, YZ, Unv5, Uuv5, Uuv6, Ubv8): New constraints. * config/mips/mips-ftypes.def: Add function types for MSA builtins. * config/mips/mips-modes.def (V16QI, V8HI, V4SI, V2DI, V4SF, V2DF) (V32QI, V16HI, V8SI, V4DI, V8SF, V4DF): New modes. * config/mips/mips-msa.md: New file. * config/mips/mips-protos.h (mips_split_128bit_const_insns): New prototype. (mips_msa_idiv_insns): Likewise. (mips_split_128bit_move): Likewise. (mips_split_128bit_move_p): Likewise. (mips_split_msa_copy_d): Likewise. (mips_split_msa_insert_d): Likewise. (mips_split_msa_fill_d): Likewise. (mips_expand_msa_branch): Likewise. (mips_const_vector_same_val_p): Likewise. (mips_const_vector_same_byte_p): Likewise. (mips_const_vector_same_int_p): Likewise. (mips_const_vector_bitimm_set_p): Likewise. (mips_const_vector_bitimm_clr_p): Likewise. (mips_msa_output_division): Likewise. (mips_ldst_scaled_shift): Likewise. (mips_expand_vec_cond_expr): Likewise. * config/mips/mips.c (mips_const_vector_bitimm_set_p): New function. (mips_const_vector_bitimm_clr_p): Likewise. (mips_const_vector_same_val_p): Likewise. (mips_const_vector_same_byte_p): Likewise. (mips_const_vector_same_int_p): Likewise. (mips_symbol_insns): Forbid loading symbols via immediate for MSA. (mips_valid_offset_p): Limit offset to 10-bit for MSA loads and stores. (mips_valid_lo_sum_p): Forbid loadings symbols via %lo(base) for MSA. (mips_lx_address_p): Add support load indexed address for MSA. (mips_address_insns): Add calculation of instructions needed for stores and loads for MSA. (mips_const_insns): Move CONST_DOUBLE below CONST_VECTOR. Handle CONST_VECTOR for MSA and let it fall through. (mips_ldst_scaled_shift): New function. (mips_subword_at_byte): Likewise. (mips_msa_idiv_insns): Likewise. (mips_legitimize_move): Validate MSA moves. (mips_rtx_costs): Add UNGE, UNGT, UNLE, UNLT cases. Add calculation of costs for MSA division. (mips_split_move_p): Check if MSA moves need splitting. (mips_split_move): Split MSA moves if necessary. (mips_split_128bit_move_p): New function. (mips_split_128bit_move): Likewise. (mips_split_msa_copy_d): Likewise. (mips_split_msa_insert_d): Likewise. (mips_split_msa_fill_d): Likewise. (mips_output_move): Handle MSA moves. (mips_expand_msa_branch): New function. (mips_print_operand): Add 'E', 'B', 'w', 'v' modifiers. Reinstate 'y' modifier. (mips_file_start): Add MSA .gnu_attribute. (mips_hard_regno_mode_ok_p): Allow TImode and 128-bit vectors in FPRs. (mips_hard_regno_nregs): Always return 1 for MSA supported mode. (mips_class_max_nregs): Add register size for MSA supported mode. (mips_cannot_change_mode_class): Allow conversion between MSA vector modes and TImode. (mips_mode_ok_for_mov_fmt_p): Allow MSA to use move.v instruction. (mips_secondary_reload_class): Force MSA loads/stores via memory. (mips_preferred_simd_mode): Add preffered modes for MSA. (mips_vector_mode_supported_p): Add MSA supported modes. (mips_autovectorize_vector_sizes): New function. (mips_msa_output_division): Likewise. (MSA_BUILTIN, MIPS_BUILTIN_DIRECT_NO_TARGET, MSA_NO_TARGET_BUILTIN): New macros. (CODE_FOR_msa_adds_s_b, CODE_FOR_msa_adds_s_h, CODE_FOR_msa_adds_s_w) (CODE_FOR_msa_adds_s_d, CODE_FOR_msa_adds_u_b, CODE_FOR_msa_adds_u_h) (CODE_FOR_msa_adds_u_w, CODE_FOR_msa_adds_u_d, CODE_FOR_msa_addv_b) (CODE_FOR_msa_addv_h, CODE_FOR_msa_addv_w, CODE_FOR_msa_addv_d) (CODE_FOR_msa_and_v, CODE_FOR_msa_bmnz_v, CODE_FOR_msa_bmz_v) (CODE_FOR_msa_bnz_v, CODE_F
RE: [PATCH][MIPS] Scheduler fix for the 74k & 24k.
Hi, > > Simon > > > > gcc/ > > * config/mips/mips.c (mips_store_data_bypass_p): Bring code into > > line with comments. > > * config/mips/sb1.md: Update usage of mips_store_data_bypass_p. > > > > This patch is OK. Committed on Simon's behalf as r226805. Regards, Robert
RE: [PATCH, MIPS] Enable load/store bonding for I6400
Ping. > -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-ow...@gcc.gnu.org] On > Behalf Of Robert Suchanek > Sent: 05 August 2015 09:31 > To: catherine_mo...@mentor.com; Matthew Fortune; gcc-patches@gcc.gnu.org > Subject: [PATCH, MIPS] Enable load/store bonding for I6400 > > Hi, > > Following up > https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01730.html > > The patch below enables the load-load/store-store bonding for MIPS32/MIPS64 > R6. > > Ok to apply? > > Regards, > Robert > > gcc/ > * config/mips/mips.h (ENABLE_LD_ST_PAIRS): Enable load/store pairs for > I6400. > --- > gcc/config/mips/mips.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h > index d17a833..6e262d6 100644 > --- a/gcc/config/mips/mips.h > +++ b/gcc/config/mips/mips.h > @@ -3177,5 +3177,5 @@ extern GTY(()) struct target_globals > *micromips_globals; > performance can be degraded for those targets. Hence, do not bond for > micromips or fix_24k. */ > #define ENABLE_LD_ST_PAIRS \ > - (TARGET_LOAD_STORE_PAIRS && TUNE_P5600 \ > + (TARGET_LOAD_STORE_PAIRS && (TUNE_P5600 || TUNE_I6400) \ > && !TARGET_MICROMIPS && !TARGET_FIX_24K) > -- > 2.4.5
RE: [PATCH, MIPS] Remove W32 and W64 pseudo-processors
Hi, > > gcc/ > > * config/mips/mips.c (mips_rtx_cost_data): Remove costs for W32 and > W64 > > pseudo-processors. > > * config/mips/mips.md (processor): Remove w32 and w64. > > OK, thanks. > > Matthew Committed as r226851. Regards, Robert
[PATCH][MIPS] Fix register renaming in the interrupt handlers
Hi,
It was discovered that with the attached test case compiled with -O2
-funroll-loops,
the regrename pass renamed one of the registers ($2) to $8 that was not
saved by the prologue.
The attached patch fixes it by defining macro HARD_REGNO_RENAME_OK that returns
zero iff the current function is an interrupt handler and a register was never
live.
Regression is still in progress. Ok to apply if it passes?
Regards,
Robert
gcc/
* config/mips/mips-protos.h (mips_hard_regno_rename_ok): New prototype.
* config/mips/mips.c (mips_hard_regno_rename_ok): New function.
* config/mips/mips.h (HARD_REGNO_RENAME_OK): New.
gcc/testsuite/
* gcc.target/mips/interrupt_handler-bug-1.c: New test.
---
gcc/config/mips/mips-protos.h | 1 +
gcc/config/mips/mips.c | 14 ++
gcc/config/mips/mips.h | 3 +++
gcc/testsuite/gcc.target/mips/interrupt_handler-bug-1.c | 12
4 files changed, 30 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/mips/interrupt_handler-bug-1.c
diff --git a/gcc/config/mips/mips-protos.h b/gcc/config/mips/mips-protos.h
index 6ce3d70..e43 100644
--- a/gcc/config/mips/mips-protos.h
+++ b/gcc/config/mips/mips-protos.h
@@ -317,6 +317,7 @@ extern unsigned int mips_sync_loop_insns (rtx, rtx *);
extern const char *mips_output_division (const char *, rtx *);
extern const char *mips_msa_output_division (const char *, rtx *);
extern const char *mips_output_probe_stack_range (rtx, rtx);
+extern bool mips_hard_regno_rename_ok (unsigned int, unsigned int);
extern unsigned int mips_hard_regno_nregs (int, machine_mode);
extern bool mips_linked_madd_p (rtx, rtx);
extern bool mips_store_data_bypass_p (rtx, rtx);
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index a9829bd..151f774 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -12959,6 +12959,20 @@ mips_hard_regno_mode_ok_p (unsigned int regno,
machine_mode mode)
return false;
}
+/* Return nonzero if register OLD_REG can be renamed to register NEW_REG. */
+
+bool
+mips_hard_regno_rename_ok (unsigned int old_reg ATTRIBUTE_UNUSED,
+ unsigned int new_reg)
+{
+ /* Interrupt functions can only use registers that have already been
+ saved by the prologue, even if they would normally be call-clobbered. */
+ if (cfun->machine->interrupt_handler_p && !df_regs_ever_live_p (new_reg))
+return false;
+
+ return true;
+}
+
/* Implement HARD_REGNO_NREGS. */
unsigned int
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 6578ae5..3fe690a 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -1932,6 +1932,9 @@ struct mips_cpu_info {
#define HARD_REGNO_MODE_OK(REGNO, MODE)
\
mips_hard_regno_mode_ok[ (int)(MODE) ][ (REGNO) ]
+#define HARD_REGNO_RENAME_OK(OLD_REG, NEW_REG) \
+ mips_hard_regno_rename_ok (OLD_REG, NEW_REG)
+
/* Select a register mode required for caller save of hard regno REGNO. */
#define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE) \
mips_hard_regno_caller_save_mode (REGNO, NREGS, MODE)
diff --git a/gcc/testsuite/gcc.target/mips/interrupt_handler-bug-1.c
b/gcc/testsuite/gcc.target/mips/interrupt_handler-bug-1.c
new file mode 100644
index 000..877d00c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/interrupt_handler-bug-1.c
@@ -0,0 +1,12 @@
+/* { dg-options "-funroll-loops" } */
+int foo;
+int bar;
+
+void __attribute__ ((interrupt)) isr(void)
+{
+ if (!foo)
+ {
+ while (bar & 0xFF30);
+ }
+}
+/* { dg-final { scan-assembler-not "\\\$8" } } */
--
2.4.5
