[OG10] Reverted patch on OG10 branch

2021-02-16 Thread Moore, Catherine
FYI - I reverted this patch:

commit ea43db9372133e84f95bdb2c6934a5cc3db3d530
Author: Catherine Moore 
Date:   Sat Feb 13 09:04:44 2021 -0800

Revert "Enable gimplify GOMP_MAP_STRUCT handling of (COMPONENT_REF 
(INDIRECT_REF ...)) map clauses."

This reverts commit bf8605f14ec33ea31233a3567f3184fee667b695.

Regressions were reported for BZ 9574 - C++ class member mapping.


[PATCH] Workaround errata for the PMC-Sierra RM7000 cpu.

2013-10-09 Thread Moore, Catherine
Hi Richard,

This patch implements a workaround for errors on the PMC-Sierra RM7000 cpu 
while executing the dmult or dmultu instruction.  The workaround is to insert 
three nops after the dmult/dmultu.

Does this look okay to commit?

Thanks,
Catherine

gcc/
2013-10-09  Catherine Moore  
Chao-ying Fu  >

* doc/invoke.texi (mfix-pmc): Document.
* config/mips/mips.md (mul3): Handle PMC errata.
(mul3_internal): Likewise.
(mul3_pmc): New pattern.
(muldi3_highpart_split): Handle PMC errata.
(mulditi3): Likewise.
(mulditi3_internal): Likewise.
(mulditi3_pmc): New pattern.
* config/mips/mips.opt (mfix-pmc): New option.

testsuite/
2013-10-09  Catherine Moore  
Chao-ying Fu  >

* gcc.target/mips/mips.exp (mips_option_groups): Append -mfix-pmc.
* gcc/target/mips/fix-pmc-[1-6].c: New tests.



pmc.patch
Description: pmc.patch


RE: [PATCH] Workaround errata for the PMC-Sierra RM7000 cpu.

2013-10-31 Thread Moore, Catherine


> -Original Message-
> From: Richard Sandiford [mailto:rdsandif...@googlemail.com]
> Sent: Thursday, October 10, 2013 1:21 PM
> To: Moore, Catherine
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Workaround errata for the PMC-Sierra RM7000 cpu.
> 
> "Moore, Catherine"  writes:
> >> -Original Message-
> >> From: Richard Sandiford [mailto:rdsandif...@googlemail.com]
> >> Subject: Re: [PATCH] Workaround errata for the PMC-Sierra RM7000 cpu.
> >>
> >> "Moore, Catherine"  writes:
> >> > Hi Richard,
> >> >
> >> > This patch implements a workaround for errors on the PMC-Sierra
> >> > RM7000 cpu while executing the dmult or dmultu instruction.  The
> >> > workaround is to insert three nops after the dmult/dmultu.
> >>
> >> Do you have any more details?  E.g. does "dmult $4,$5;addiu $6,$7,1"
> >> cause problems?  The VR41xx series had errata like these, but it was
> >> always between the multiplication and other references to HI/LO.
> >
> > I've attached documentation describing the errata.  The problem occurs
> > with dmult/dmultu followed by a load.
> 
> Thanks.
> 
> >> Normally we've handled this by getting the assembler to insert the
> >> nops and compiling in .set reorder mode.  That copes with inline asms too.
> >> I'm not opposed to doing it differently if there's a specific reason 
> >> though.
> >
> > No specific reason.  Let me know if we need to change the
> implementation.
> 
> The assembler's probably best, all other things being equal.
> 
> Ideally opcodes/mips*.c would have a flag to mark load instructions, but all
> we have at the moment are flags to mark load delays, which means that LD
> and some other load instructions don't have them.  So I can think of three
> options:
> 
> (1) Turn INSN_LOAD_MEMORY_DELAY into INSN_LOAD_GPR and
> INSN_COPRO_MEMORY_DELAY into INSN_LOAD_COPRO.  Add the flags to
> all
> loads in the opcode table that don't currently have one.
> 
> This would be the best approach, but I can understand why it might
> seem like too much effort.
> 

I've decided to go with this approach and will post the patches to the binutils 
mailing list.  I'll follow up here with the recognition of the mfix-pmc option 
by the driver program.

> (2) Stick with inserting three nops regardless.  This could be done by
> making insns_between return 3 for insn1 being DMULT(U), regardless
> of insn2.
> 
> (3) A slightly hacky approach would be to do (2), but check for memory
> instructions using:
> 
> (insn2 == NULL || strchr (insn2->insn_mo->args, '('))
> 
> That would conservatively match prefetches and stores too, but would
> at least avoid the nops in some cases.  It's probably not worth the
> risk or hackiness though.
> 
> Thanks,
> Richard


RE: [PATCH] MIPS: Prevent the p5600-bonding.c test from being run for the n32 and 64 ABIs

2016-01-27 Thread Moore, Catherine


> -Original Message-
> From: Andrew Bennett [mailto:andrew.benn...@imgtec.com]
> Sent: Wednesday, January 20, 2016 9:42 AM
> To: Matthew Fortune; gcc-patches@gcc.gnu.org
> Cc: Moore, Catherine
> Subject: RE: [PATCH] MIPS: Prevent the p5600-bonding.c test from being run
> for the n32 and 64 ABIs
> 
> Ping.
> 

This is OK now.

> 
> > -Original Message-
> > From: gcc-patches-ow...@gcc.gnu.org
> > [mailto:gcc-patches-ow...@gcc.gnu.org] On Behalf Of Andrew Bennett
> > Sent: 02 September 2015 14:55
> > To: Matthew Fortune; gcc-patches@gcc.gnu.org
> > Cc: Moore, Catherine (catherine_mo...@mentor.com)
> > Subject: RE: [PATCH] MIPS: Prevent the p5600-bonding.c test from being
> > run for the n32 and 64 ABIs
> >
> > > > diff --git a/gcc/testsuite/gcc.target/mips/p5600-bonding.c
> > > > b/gcc/testsuite/gcc.target/mips/p5600-bonding.c
> > > > index 0890ffa..20c26ca 100644
> > > > --- a/gcc/testsuite/gcc.target/mips/p5600-bonding.c
> > > > +++ b/gcc/testsuite/gcc.target/mips/p5600-bonding.c
> > > > @@ -1,6 +1,7 @@
> > > >  /* { dg-do compile } */
> > > >  /* { dg-options "-dp -mtune=p5600  -mno-micromips -mno-mips16" }
> > > > */
> > > >  /* { dg-skip-if "Bonding needs peephole optimization." { *-*-* } { 
> > > > "-O0"
> > "-
> > > O1" } { "" } }
> > > > */
> > > > +/* { dg-skip-if "There is no DI mode support for load/store
> > > > +bonding" { *-
> > *-
> > > * } { "-
> > > > mabi=n32" "-mabi=64" } { "" } } */  typedef int VINT32
> > > > __attribute__ ((vector_size((16;
> > >
> > > If the best fix we can do for this test is to limit what it tests
> > > then we should still not just skip it. There is some precedence for
> > > tests that require a specific arch with the isa=loongson special
> > > case. I'd rather just lock the test down to p5600 as per the filename.
> >
> > I have changed the testcase's dg-options so that it is only built for p5600.
> > The updated patch and ChangeLog are below.
> >
> > Ok to commit?
> >
> > Many thanks,
> >
> >
> >
> > Andrew
> >
> >
> > testsuite/
> > * gcc.target/mips/p5600-bonding.c (dg-options): Force the test to be
> > always
> > built for p5600.
> > * gcc.target/mips/mips.exp (mips-dg-options): Add support for the
> > isa=p5600
> > dg-option.
> >
> >
> > diff --git a/gcc/testsuite/gcc.target/mips/mips.exp
> > b/gcc/testsuite/gcc.target/mips/mips.exp
> > index 42e7fff..e8d1895 100644
> > --- a/gcc/testsuite/gcc.target/mips/mips.exp
> > +++ b/gcc/testsuite/gcc.target/mips/mips.exp
> > @@ -142,6 +142,9 @@
> >  #   isa=loongson
> >  #  select a Loongson processor
> >  #
> > +#   isa=p5600
> > +#  select a P5600 processor
> > +#
> >  #   addressing=absolute
> >  #  force absolute addresses to be used
> >  #
> > @@ -1009,6 +1012,10 @@ proc mips-dg-options { args } {
> > if { ![regexp {^-march=loongson} $arch] } {
> > set arch "-march=loongson2f"
> > }
> > +   } elseif { [string equal $spec "isa=p5600"] } {
> > +   if { ![regexp {^-march=p5600} $arch] } {
> > +   set arch "-march=p5600"
> > +   }
> > } else {
> > if { ![regexp {^(isa(?:|_rev))(=|<=|>=)([0-9]*)$} \
> >$spec dummy prop relation value nocpus] } {
> > diff --git a/gcc/testsuite/gcc.target/mips/p5600-bonding.c
> > b/gcc/testsuite/gcc.target/mips/p5600-bonding.c
> > index 0890ffa..0bc6d91 100644
> > --- a/gcc/testsuite/gcc.target/mips/p5600-bonding.c
> > +++ b/gcc/testsuite/gcc.target/mips/p5600-bonding.c
> > @@ -1,5 +1,5 @@
> >  /* { dg-do compile } */
> > -/* { dg-options "-dp -mtune=p5600  -mno-micromips -mno-mips16" } */
> > +/* { dg-options "-dp isa=p5600 -mtune=p5600 -mno-micromips
> > +-mno-mips16" } */
> >  /* { dg-skip-if "Bonding needs peephole optimization." { *-*-* } {
> > "-O0" "- O1" } { "" } } */  typedef int VINT32 __attribute__
> > ((vector_size((16;



RE: [Patch, MIPS] Patch for PR 68400, a mips16 bug

2016-01-28 Thread Moore, Catherine


> -Original Message-
> From: Steve Ellcey [mailto:sell...@imgtec.com]
> Sent: Tuesday, January 26, 2016 3:43 PM
> To: gcc-patches@gcc.gnu.org
> Cc: matthew.fort...@imgtec.com; Moore, Catherine
> Subject: [Patch, MIPS] Patch for PR 68400, a mips16 bug
> 
> Here is a patch for PR6400.  The problem is that and_operands_ok was
> checking one operand to see if it was a memory_operand but MIPS16
> addressing is more restrictive than what the general memory_operand
> allows.  The fix was to call mips_classify_address if TARGET_MIPS16 is set
> because it will do a more complete mips16 addressing check and reject
> operands that do not meet the more restrictive requirements.
> 
> I ran the GCC testsuite with no regressions and have included a test case as
> part of this patch.
> 
> OK to checkin?
> 
> Steve Ellcey
> sell...@imgtec.com
> 
> 
> 2016-01-26  Steve Ellcey  
> 
>   PR target/68400
>   * config/mips/mips.c (and_operands_ok): Add MIPS16 check.
> 
> 
> 
This is OK.
Thanks, Catherine


microMIPS jump instructions

2014-01-08 Thread Moore, Catherine
Hi Richard,

It looks like the microMIPS implementation is missing support for the JRC 
instruction and also misses an opportunity to generate JALS.
I've attached a patch, plus some new test cases to correct this.  Does this 
look okay to commit?  I'd like to get it in 4.9.

Thanks,
Catherine



jrc-jals.cl
Description: jrc-jals.cl


jrc-jals.patch
Description: jrc-jals.patch


RE: [PATCH, MIPS] Support interrupt handlers with hard-float

2015-07-13 Thread Moore, Catherine


> -Original Message-
> From: Robert Suchanek [mailto:robert.sucha...@imgtec.com]
> Sent: Wednesday, July 08, 2015 6:42 AM
> To: Matthew Fortune; Moore, Catherine; gcc-patches@gcc.gnu.org
> Subject: [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?

Yes, this one is OK.

> 
> 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.



RE: [PATCH, MIPS] Support new interrupt handler options

2015-07-13 Thread Moore, Catherine


> -Original Message-
> From: Robert Suchanek [mailto:robert.sucha...@imgtec.com]
> Sent: Wednesday, July 08, 2015 6:43 AM
> To: Matthew Fortune; Moore, Catherine; gcc-patches@gcc.gnu.org
> Subject: [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.

Hi Robert,
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 a couple of further comments on the existing patch, see below.

Thanks,
Catherine

> 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
> @@ -1325,13 +1359,62 @@ mips_interrupt_type_p (tree type)
>return lookup_attribute ("interrupt", TYPE_ATTRIBUTES (type)) != NULL;  }
> 
> +static enum mips_int_mask
> +mips_interrupt_mask (tree type)

This function requires a comment.

> +static enum mips_shadow_set
> +mips_use_shadow_register_set (tree type)

Likewise.

>  {
> @@ -1537,6 +1620,87 @@ mips_can_inline_p (tree caller, tree callee)
>  return false;
>return default_target_can_inline_p (caller, callee);  }
> +
> +static tree
> +mips_handle_interrupt_attr (tree *node, tree name, tree args,
> + int flags ATTRIBUTE_UNUSED, bool *no_add_attrs)
> {
Likewise.

> +
> +static tree
> +mips_handle_use_shadow_register_set_attr (tree *node, tree name, tree
> args,

And here as well.
> 



RE: [PATCH, MIPS] Fix restoration of hi/lo in MIPS64R2 interrupt handlers

2015-07-13 Thread Moore, Catherine


> -Original Message-
> From: Robert Suchanek [mailto:robert.sucha...@imgtec.com]
> Sent: Wednesday, July 08, 2015 6:43 AM
> To: Moore, Catherine; Matthew Fortune; gcc-patches@gcc.gnu.org
> Subject: [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?
> 
> gcc/
>   * config/mips/mips.c (mips_emit_save_slot_move): Fix typo.
> 
> gcc/testsuite/
>   * gcc.target/mips/20150707.c: New test.

Hi Robert,
The patch is OK, but will you please name the test something other than the 
date?
Thanks,
Catherine


RE: [PATCH, MIPS] Support new interrupt handler options

2015-07-14 Thread Moore, Catherine


> -Original Message-
> From: Robert Suchanek [mailto:robert.sucha...@imgtec.com]
> Sent: Tuesday, July 14, 2015 11:14 AM
> To: Moore, Catherine; Matthew Fortune; gcc-patches@gcc.gnu.org
> Subject: 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 re-ran patch with the verbose option and found that it was silently 
discarding hunks (starting with #7) because it thought it was garbage.

> 
> >
> > 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.

This is now OK to commit.
Catherine



RE: [PATCH] MIPS: Correctly update the isa and arch_test_option_p variables after the arch dependency handling code in mips.exp

2015-07-14 Thread Moore, Catherine


> -Original Message-
> From: Andrew Bennett [mailto:andrew.benn...@imgtec.com]
> Sent: Tuesday, July 14, 2015 9:24 AM
> To: Richard Sandiford; Matthew Fortune
> Cc: gcc-patches@gcc.gnu.org; Moore, Catherine
> Subject: RE: [PATCH] MIPS: Correctly update the isa and arch_test_option_p
> variables after the arch dependency handling code in mips.exp
> 
> > Yeah, I agree that this doesn't really fit the model that well, but
> > like you say, we're stretching the logic a bit :-).  When I wrote it,
> > the architectures formed a nice tree in which moving to leaf nodes
> > only added features.  So in the pre-r6 days:
> >
> > # Handle dependencies between the pre-arch options and the arch
> option.
> > # This should mirror the arch and post-arch code below.
> > if { !$arch_test_option_p } {
> >
> > increased the architecture from the --target_board default to match
> > the features required by the test, whereas:
> >
> > # Handle dependencies between the arch option and the post-arch
> options.
> > # This should mirror the arch and pre-arch code above.
> > if { $arch_test_option_p } {
> >
> > turned off features from the --target_board default to match a lower
> > architecture required by the test.  So in the pre-r6 days, all the
> > code in the second block was turning something off when going to a
> > lower architecture.  The blocks were mutually-exclusive and writing it
> > this way reduced the number of redundant options.  (Admittedly you
> > could argue that it's daft to worry about that given the kind of
> > command lines you tend to get from the rest of mips.exp. :-))
> >
> > r6 is the first time we've had to turn something off when moving up.
> > -mnan and -mabs are also the first options where old architectures
> > support only A, higher revisions support A and B, and the newest
> > revision supports only B.  I think I'd prefer to acknowledge that and
> > have:
> >
> > # Handle dependencies between the arch option and the post-arch
> options.
> > # This should mirror the arch and pre-arch code above.  For pre-r6
> > # architectures this only needs to be done when we've moved down
> > # to a lower architecture and might need to turn features off,
> > # but moving up from pre-r6 to r6 can remove features too.
> > if { $arch_test_option_p || ($orig_isa_rev < 6 && $isa_rev >= 6) }
> > {
> >
> > I think the existing r6->r5 case really is different: there we're
> > forcing a -mnan option not because the architecture needs it but
> > because the environment might.
> 
> Hi,
> 
> An updated patch and ChangeLog which reflects the comments is below.
> I have tested it on the mips-img-elf and mips-mti-elf toolchains with all the
> valid multilib configurations and there are no new regressions.
> 
> Ok to commit?

Yes, this is OK.
> 
> 
> testsuite/
>   * gcc.target/mips/mips.exp (mips-dg-options): Allow the post-arch
>   code to be run when the pre-arch code increases the isa_rev to
>   mips32r6 or greater.
> 
> 




RE: [PATCH, MIPS] Add -march=interaptiv

2015-07-17 Thread Moore, Catherine


> -Original Message-
> From: Robert Suchanek [mailto:robert.sucha...@imgtec.com]
> Sent: Thursday, July 16, 2015 10:17 AM
> Subject: [PATCH, MIPS] Add -march=interaptiv
> 
> As in the title, the attached patch adds -march=interaptiv defined to 24kf2_1,
> mapped to -mips32r2 and -mdsp.
> 
> OK to apply?
> 
> 
> 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.


RE: [PATCH][MIPS] Scheduler fix for the 74k & 24k.

2015-07-31 Thread Moore, Catherine


> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Simon Dardis
> Sent: Tuesday, July 21, 2015 6:39 AM
> To: gcc-patches@gcc.gnu.org
> Subject: [PATCH][MIPS] Scheduler fix for the 74k & 24k.
> 
> Hello,
> 
> This patch fixes a bug with the 74k & 24k schedulers.
> 
> Back in 2006  (2ca4dfa486bd358c6e466328839977250d160393) a
> mips_store_data_bypass_p was added to the mips backend. Unfortunately
> it was defined in terms of !store_data_bypass_p, though it was correctly
> used for the sb1 processor pipeline descriptor at that time. Later during a
> code-cleanup in 2012 (e053750d33e14ca245e14e1c467709a9bf6c6282) the 24k
> & 74k bypasses were changed from the correct !store_data_bypass_p to
> !mips_store_data_bypass_p. This lead to those bypasses having inverted
> guard conditions.
> 
> This patch brings mips_store_data_bypass_p into line with its comments and
> the comments of store_data_bypass_p. It also corrects the sb1's pipeline
> description.
> 
> Thanks,
> 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.


RE: [PATCH, MIPS, Ping] Inline memcpy for MipsR6

2015-08-01 Thread Moore, Catherine


> -Original Message-
> From: Simon Dardis [mailto:simon.dar...@imgtec.com]
> Sent: Wednesday, July 29, 2015 4:29 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Moore, Catherine
> Subject: [PATCH, MIPS, Ping] Inline memcpy for MipsR6
> 
> > This patch enables inline memcpy for R6 which was previously disabled and
> adds support for expansion when source and destination are at least half-
> word aligned.
> 
> https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00749.html
> 

Hi Simon,

Two things need to be fixed up with this patch before committing.

1.  The new test inline-memcpy-2.c should not be run with -OS (like the other 
new tests that you submitted).

2.  Your patch is against older source than what is currently in the 
repository, causing this hunk not to apply cleanly:

@@ -8311,8 +8321,8 @@ bool
 mips_expand_block_move (rtx dest, rtx src, rtx length)
 {
   if (!ISA_HAS_LWL_LWR
-   && (MEM_ALIGN (src) < BITS_PER_WORD
-  || MEM_ALIGN (dest) < BITS_PER_WORD))
+  && (MEM_ALIGN (src) < MIPS_MIN_MOVE_MEM_ALIGN
+ || MEM_ALIGN (dest) < MIPS_MIN_MOVE_MEM_ALIGN))
 return false;

   if (CONST_INT_P (length))


The correct patch should like this:

@@ -7780,8 +7790,9 @@
 bool
 mips_expand_block_move (rtx dest, rtx src, rtx length)
 {
-  /* Disable entirely for R6 initially.  */
-  if (!ISA_HAS_LWL_LWR)
+  if (!ISA_HAS_LWL_LWR
+  && (MEM_ALIGN (src) < MIPS_MIN_MOVE_MEM_ALIGN
+ || MEM_ALIGN (dest) < MIPS_MIN_MOVE_MEM_ALIGN))
 return false;

   if (CONST_INT_P (length))

Okay with those changes.
Thanks,
Catherine


RE: [patch] libstdc++/65142 Check read() result in std::random_device.

2015-09-17 Thread Moore, Catherine


> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Jonathan Wakely
> Sent: Thursday, September 17, 2015 5:28 PM
> To: Gerald Pfeifer
> Cc: libstd...@gcc.gnu.org; gcc-patches@gcc.gnu.org
> Subject: Re: [patch] libstdc++/65142 Check read() result in
> std::random_device.
> 
> On 17/09/15 22:21 +0200, Gerald Pfeifer wrote:
> >On Thu, 17 Sep 2015, Jonathan Wakely wrote:
> >>> Any comments on this version?
> >> Committed to trunk.
> >
> >Unfortunately this broke bootstrap on FreeBSD 10.1.
> >
> >/scratch/tmp/gerald/gcc-HEAD/libstdc++-v3/src/c++11/random.cc: In
> member function 'std::random_device::result_type
> std::random_device::_M_getval()':
> >/scratch/tmp/gerald/gcc-HEAD/libstdc++-v3/src/c++11/random.cc:144:22:
> >error: 'errno' was not declared in this scope
> >  else if (e != -1 || errno != EINTR)
> >  ^
> >/scratch/tmp/gerald/gcc-HEAD/libstdc++-v3/src/c++11/random.cc:144:31:
> >error: 'EINTR' was not declared in this scope
> >  else if (e != -1 || errno != EINTR)
> >   ^
> >Makefile:545: recipe for target 'random.lo' failed
> >
> >I probably won't be able to dig in deeper today, but figured this might
> >already send you on the right path?
> >
> >Actually...
> >
> >...how about he patch below?  Bootstraps on i386-unknown-freebsd10.1,
> >no regressions.
> 
> Sorry about that, I've committed your patch.
> 
> >Gerald
> >
> >
I'm still seeing errors for a build of the mips-sde-elf target with these 
patches.

Errors are:
/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/src/c++11/debug.cc:
 In function 'void {anonymous}::print_word({anonymous
}::PrintContext&, const char*, std::ptrdiff_t)':
/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/src/c++11/debug.cc:573:10:
 error: 'stderr' was not declared in this scop
e
  fprintf(stderr, "\n");
  ^
/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/src/c++11/debug.cc:573:22:
 error: 'fprintf' was not declared in this sco
pe
  fprintf(stderr, "\n");
  ^
/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/src/c++11/debug.cc:596:14:
 error: 'stderr' was not declared in this scop
e
  fprintf(stderr, "%s", spacing);
  ^
/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/src/c++11/debug.cc:596:35:
 error: 'fprintf' was not declared in this sco
pe
  fprintf(stderr, "%s", spacing);
   ^
/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/src/c++11/debug.cc:600:24:
 error: 'stderr' was not declared in this scope
  int written = fprintf(stderr, "%s", word);
^
/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/src/c++11/debug.cc:600:42:
 error: 'fprintf' was not declared in this scop
e
  int written = fprintf(stderr, "%s", word);
  ^

Thanks,
Catherine



RE: [patch] libstdc++/65142 Check read() result in std::random_device.

2015-09-17 Thread Moore, Catherine


> -Original Message-
> From: Jonathan Wakely [mailto:jwak...@redhat.com]
> Sent: Thursday, September 17, 2015 6:54 PM
> To: Moore, Catherine; fdum...@gcc.gnu.org
> Cc: Gerald Pfeifer; libstd...@gcc.gnu.org; gcc-patches@gcc.gnu.org
> Subject: Re: [patch] libstdc++/65142 Check read() result in
> std::random_device.
> 
> On 17/09/15 22:32 +, Moore, Catherine wrote:
> >
> >
> >> -Original Message-
> >> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> >> ow...@gcc.gnu.org] On Behalf Of Jonathan Wakely
> >> Sent: Thursday, September 17, 2015 5:28 PM
> >> To: Gerald Pfeifer
> >> Cc: libstd...@gcc.gnu.org; gcc-patches@gcc.gnu.org
> >> Subject: Re: [patch] libstdc++/65142 Check read() result in
> >> std::random_device.
> >>
> >> On 17/09/15 22:21 +0200, Gerald Pfeifer wrote:
> >> >On Thu, 17 Sep 2015, Jonathan Wakely wrote:
> >> >>> Any comments on this version?
> >> >> Committed to trunk.
> >> >
> >> >Unfortunately this broke bootstrap on FreeBSD 10.1.
> >> >
> >> >/scratch/tmp/gerald/gcc-HEAD/libstdc++-v3/src/c++11/random.cc: In
> >> member function 'std::random_device::result_type
> >> std::random_device::_M_getval()':
> >> >/scratch/tmp/gerald/gcc-HEAD/libstdc++-
> v3/src/c++11/random.cc:144:22:
> >> >error: 'errno' was not declared in this scope
> >> >  else if (e != -1 || errno != EINTR)
> >> >  ^
> >> >/scratch/tmp/gerald/gcc-HEAD/libstdc++-
> v3/src/c++11/random.cc:144:31:
> >> >error: 'EINTR' was not declared in this scope
> >> >  else if (e != -1 || errno != EINTR)
> >> >   ^
> >> >Makefile:545: recipe for target 'random.lo' failed
> >> >
> >> >I probably won't be able to dig in deeper today, but figured this
> >> >might already send you on the right path?
> >> >
> >> >Actually...
> >> >
> >> >...how about he patch below?  Bootstraps on
> >> >i386-unknown-freebsd10.1, no regressions.
> >>
> >> Sorry about that, I've committed your patch.
> >>
> >> >Gerald
> >> >
> >> >
> >I'm still seeing errors for a build of the mips-sde-elf target with these
> patches.
> >
> >Errors are:
> >/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/s
> >rc/c++11/debug.cc: In function 'void
> {anonymous}::print_word({anonymous
> >}::PrintContext&, const char*, std::ptrdiff_t)':
> >/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/s
> >rc/c++11/debug.cc:573:10: error: 'stderr' was not declared in this scop
> >e
> >  fprintf(stderr, "\n");
> >  ^
> >/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/s
> >rc/c++11/debug.cc:573:22: error: 'fprintf' was not declared in this sco
> >pe
> >  fprintf(stderr, "\n");
> >  ^
> >/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/s
> >rc/c++11/debug.cc:596:14: error: 'stderr' was not declared in this scop e
> >  fprintf(stderr, "%s", spacing);
> >  ^
> >/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/s
> >rc/c++11/debug.cc:596:35: error: 'fprintf' was not declared in this sco pe
> >  fprintf(stderr, "%s", spacing);
> >   ^
> >/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/s
> >rc/c++11/debug.cc:600:24: error: 'stderr' was not declared in this
> >scope
> >  int written = fprintf(stderr, "%s", word);
> >^
> >/scratch/cmoore/virgin-upstream-elf-lite/src/gcc-trunk-5/libstdc++-v3/s
> >rc/c++11/debug.cc:600:42: error: 'fprintf' was not declared in this
> >scop e
> >  int written = fprintf(stderr, "%s", word);
> 
> That's a different problem, due to https://gcc.gnu.org/r227885
> 
> François, could you take a look please?
> 

I've now committed this patch to solve this problem (pre-approved by Jonathan).

2015-09-17  Catherine Moore  

* src/c++11/debug.cc: Include .


Index: src/c++11/debug.cc
===
--- src/c++11/debug.cc  (revision 227887)
+++ src/c++11/debug.cc  (working copy)
@@ -32,6 +32,7 @@
 #include 

 #include 
+#include 

 #include  // for std::min
 #include  // for _Hash_impl


RE: [PATCH, MIPS] Frame header optimization for MIPS O32 ABI

2015-09-28 Thread Moore, Catherine


> -Original Message-
> From: Steve Ellcey [mailto:sell...@imgtec.com]
> Sent: Friday, September 11, 2015 2:06 PM
> To: Matthew Fortune
> Cc: GCC Patches; Moore, Catherine
> Subject: RE: [PATCH, MIPS] Frame header optimization for MIPS O32 ABI
> 
> On Fri, 2015-09-04 at 01:40 -0700, Matthew Fortune wrote:
> 
> > A few comments below. I found some of the comments a bit hard to parse
> but have
> > not attempted any rewording. I'd like Catherine to comment too as I have
> barely
> > any experience at the gimple level to know if this accounts for any
> necessary
> > subtleties.
> 
> Catherine said she would look at this next week but I have updated the
> patch in the mean time to address your comments and give Catherine a
> more up-to-date patch to look over.
> 

Hi Steve, I'm sorry for the delay in reviewing this patch. 
Some changes have been committed upstream (see revision #227941) that will 
require updates to this patch.
Please post the update for review.  Other comments are embedded.

> 
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 5712547..eea97de 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -420,6 +420,7 @@ microblaze*-*-*)
>  mips*-*-*)
>   cpu_type=mips
>   extra_headers="loongson.h"
> + extra_objs="frame-header-opt.o"
>   extra_options="${extra_options} g.opt fused-madd.opt mips/mips-
> tables.opt"
>   ;;
>  nds32*)
> diff --git a/gcc/config/mips/frame-header-opt.c b/gcc/config/mips/frame-
> header-opt.c
> index e69de29..5c4e93c 100644
> --- a/gcc/config/mips/frame-header-opt.c
> +++ b/gcc/config/mips/frame-header-opt.c
> @@ -0,0 +1,221 @@
> +/* Analyze functions to determine if calling functions need to allocate
> +   stack space (a frame header) for its called functions to write out their
> +   arguments on to the stack.  This optimization is only applicable to
> +   TARGET_OLDABI targets because calling functions on TARGET_NEWABI
> targets
> +   do not allocate any stack space for arguments (the called function does it
> +   if needed).
> +

Overall, I agree with Matthew regarding the comments being a little hard to 
parse.
How about:

/* Analyze functions to determine if callers need to allocate a frame header on 
the stack.  The frame header is used by callees to save its arguments.
   This optimization is specific to TARGET_OLDABI targets.  For TARGET_NEWABI 
targets, if a frame header is required, it is allocated by the callee.  */


> +
> +/* Look at all the functions this function calls and return true if none of
> +   them need the argument stack space that this function would normally
> +   allocate.  Return false if one or more functions does need this space
> +   or if we cannot determine that all called functions do not need the
> +   space.  */

/* Returns TRUE if the argument stack space allocated by function FN is used.
 Returns FALSE if the space is needed or if the need for the space cannot 
be determined.  */
> +
> +static bool
> +}
> +
> +/* This optimization scans all the functions in the compilation unit to find
> +   out which ones do not need the frame header that their caller normally
> +   allocates.  Then it does a second scan of all the functions to determine
> +   which functions can skip the allocation because none of the functions it
> +   calls need the frame header.  */
> +

  /* Scan each function to determine those that need its frame headers.  
Perform a second
   scan to determine if the allocation can be skipped because none of its 
callees require the frame header.  */
> +}
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index c0ec0fd..8152645 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -17940,6 +17941,18 @@ if @var{ra-address} is nonnull.
> 
>  The default is @option{-mno-mcount-ra-address}.
> 
> +@item -mframe-header-opt
> +@itemx -mno-frame-header-opt
> +@opindex mframe-header-opt
> +Enable (disable) frame header optimization in the o32 ABI.  When using
> +the o32 ABI, calling functions allocate 16 bytes on the stack in case
> +the called function needs to write out register arguments to memory so
> +that their address can be taken.  When enabled, this optimization will
> +cause the calling function to not allocate that space if it can determine
> +that none of its called functions use it.
> +
> +This optimization is off by default at all optimization levels.
> +
>  @end table
> 
>  @node MMIX Options

How about this instead:
Enable (disable) frame header optimization in the o32 ABI.  When using the o32
ABI, calling functions will allocate 16 bytes on the stack for the called 
function
to write out register arguments.  When enabled, this optimization will suppress 
the
allocation of the frame header if it can be determined that it is unused.

This optimization is off by default at all optimization levels.

Catherine


RE: [RFA] Compact EH Patch

2015-10-05 Thread Moore, Catherine

> -Original Message-
> From: Richard Henderson [mailto:r...@redhat.com]
> Subject: Re: [RFA] Compact EH Patch
> 

Richard, Thanks for the patch review.  
Matthew, Would you please take a look at the MIPS-specific pieces before I 
resubmit the patch?
There will be a small change to the data encoding for the MIPS-Linux toolchain 
to fix a problem that we found in the mabi=64 libraries.
 A follow-on patch to binutils will also be required.  I'll do an update to 
this patch, soon, including any changes for the MIPS backend.

Please see the embedded comments and a couple of questions.

Thanks,
Catherine

> > Index: libgcc/libgcc-std.ver.in
> >
> ==
> =
> > --- libgcc/libgcc-std.ver.in(revision 226409)
> > +++ libgcc/libgcc-std.ver.in(working copy)
> > @@ -1918,6 +1918,7 @@ GCC_4.6.0 {
> >__morestack_current_segment
> >__morestack_initial_sp
> >__splitstack_find
> > +  _Unwind_GetEhEncoding
> >  }
> >
> >  %inherit GCC_4.7.0 GCC_4.6.0
> > @@ -1938,3 +1939,8 @@ GCC_4.7.0 {
> >  %inherit GCC_4.8.0 GCC_4.7.0
> >  GCC_4.8.0 {
> >  }
> > +
> > +%inherit GCC_4.8.0 GCC_4.7.0
> > +GCC_4.8.0 {
> > +  __register_frame_info_header_bases
> > +}
> 
> You can't push new symbols into old versions.  These have to go into the
> version for the current gcc.

Yep, okay.  I'll fix these in the next version.
> 
> Likewise.
> 
> > +  if (data.type != CET_not_found)
> > +return data.type;
> > +
> > +  return CET_not_found;
> 
> Return data.type unconditionally.

OK.
> 
> > +++ libgcc/unwind-pe.h  (working copy)
> > @@ -44,6 +44,7 @@
> >  #define DW_EH_PE_udata2 0x02
> >  #define DW_EH_PE_udata4 0x03
> >  #define DW_EH_PE_udata8 0x04
> > +#define DW_EH_PE_udata1 0x05
> >  #define DW_EH_PE_sleb1280x09
> >  #define DW_EH_PE_sdata2 0x0A
> >  #define DW_EH_PE_sdata4 0x0B
> 
> If we're going to add udata1, we might as well add sdata1 for consistency.

OK.
> 
> > @@ -184,6 +187,7 @@ read_encoded_value_with_base (unsigned char
> encodi
> >union unaligned
> >  {
> >void *ptr;
> > +  unsigned u1 __attribute__ ((mode (QI)));
> >unsigned u2 __attribute__ ((mode (HI)));
> >unsigned u4 __attribute__ ((mode (SI)));
> >unsigned u8 __attribute__ ((mode (DI)));
> 
> This is silly.  Access to a single byte never requires alignment help from the
> compiler...

OK.
> 
> > +   case DW_EH_PE_udata1:
> > + result = u->u1;
> > + p += 1;
> > + break;
> 
> result = *p;
> 
> > +  read_encoded_value_with_base (DW_EH_PE_absptr |
> DW_EH_PE_udata4, 0,
> > +   hdr + 4, &nrec);
> 
> Err... that encoding type makes no sense: absptr is "pointer size".  Combining
> that with an explicit size, like udata4, means that udata4 wins.  So this is 
> the
> same as simply writing DW_EH_PE_udata4.
> 
> At which point you're loading a 4 byte unsigned quantity from an aligned
> address.  You might as well use *(uint32_t *)(hdr + 4).
> 

Ok, here as well.

> > +__register_frame_info_header_bases (const void *begin, struct object
> *ob,
> > +   void *tbase, void *dbase)
> > +{
> > +  /* Only register compact EH frame headers.  */
> > +  if (begin == NULL || *(const char *) begin != 2)
> > +return;
> 
> Check for no entries before registering?

That's reasonable.

> 
> > +extern char __GNU_EH_FRAME_HDR[] TARGET_ATTRIBUTE_WEAK;
> 
> Don't you have some guaranteed alignment for this table?
> That perhaps ought to be seen in either the type or an attribute.
> 

OK.
> +  if (op < 0x40)
> +  else if (op < 0x48)
> +  else if (op < 0x50)
> +  else if (op < 0x58)
> +  else if (op == 0x58)
> +  else if (op == 0x59)
> +  else if (op == 0x5a)
> +  else if (op == 0x5b)
> +  else if (op == 0x5c)
> +  else if (op == 0x5d)
> +  else if (op == 0x5e)
> +  else if (op == 0x5f)
> +  else if (op >= 0x60 && op < 0x6c)
> 
> Better as a switch statement surely, using the gcc case x ... y: extension.
OK.
> 
> > +static _Unwind_Reason_Code
> > +uw_frame_state_compact (struct _Unwind_Context *context,
> > +   _Unwind_FrameState *fs,
> > +   enum compact_entry_type entry_type,
> > +   struct compact_eh_bases *bases) {
> > +  const unsigned char *p;
> > +  unsigned int pr_index;
> > +  _Unwind_Ptr personality;
> > +  unsigned char buf[4];
> > +  _Unwind_Reason_Code rc;
> > +
> > +  p = bases->entry;
> > +  pr_index = *(p++);
> > +  switch (pr_index) {
> > +  case 0:
> > +  p = read_encoded_value (context, bases->eh_encoding, p,
> &personality);
> > +  fs->personality = (_Unwind_Personality_Fn) personality;
> > +  break;
> > +  case 1:
> > +  fs->personality = __gnu_compact_pr1;
> > +  break;
> > +  case 2:
> > +  fs->personality = __gnu_compact_pr2;
> > +  break;
> > +  default:
> > +  fs->personality = NU

RE: [PATCH, MIPS] Frame header optimization for MIPS O32 ABI

2015-10-05 Thread Moore, Catherine


> -Original Message-
> From: Steve Ellcey [mailto:sell...@imgtec.com]
> Sent: Monday, October 05, 2015 1:16 PM
> To: Moore, Catherine
> Cc: Matthew Fortune; GCC Patches
> Subject: RE: [PATCH, MIPS] Frame header optimization for MIPS O32 ABI
> 
> On Mon, 2015-09-28 at 22:10 +, Moore, Catherine wrote:
> 
> > Hi Steve, I'm sorry for the delay in reviewing this patch.
> > Some changes have been committed upstream (see revision #227941) that
> > will require updates to this patch.
> > Please post the update for review.  Other comments are embedded.
> 
> OK, I have updated the comments based on your input and changed the
> code to compile with the ToT GCC after revision @227941.  Here is the new
> patch.
> 

HI Sreve,

The patch itself looks good, but the tests that you added need a little more 
work.

I tested with the mips-sde-elf-lite configuration and I'm seeing failures for 
many options.  The main failure mode seems to be that the stack is incremented 
by 24 instead of 32.
I tried this change in frame-header-1.c and frame-header-3.c:

/* { dg-final { scan-assembler-not "\taddiu\t\\\$sp,\\\$sp,-8" } } 

instead of:

/* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-32" }

There are still errors in frame-header-2.c when compiling with -mips16 and 
-mabi=64 (this one uses a daddiu).
Also, the tests fail for -flto variants.

Would you please fix this up and resubmit?

Thanks,
Catherine

> 
> 
> diff --git a/gcc/testsuite/gcc.target/mips/frame-header-1.c
> b/gcc/testsuite/gcc.target/mips/frame-header-1.c
> new file mode 100644
> index 000..8495e0f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/frame-header-1.c
> @@ -0,0 +1,21 @@
> +/* Verify that we do not optimize away the frame header in foo when using
> +   -mno-frame-header-opt by checking the stack pointer increment done in
> +   that function.  Without the optimization foo should increment the stack
> +   by 32 bytes, with the optimization it would only be 8 bytes.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-mno-frame-header-opt" } */
> +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> +/* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-32" } } */
> +
> +void __attribute__((noinline))
> +bar (int* a)
> +{
> +  *a = 1;
> +}
> +
> +void
> +foo (int a)
> +{
> +  bar (&a);
> +}
> diff --git a/gcc/testsuite/gcc.target/mips/frame-header-2.c
> b/gcc/testsuite/gcc.target/mips/frame-header-2.c
> new file mode 100644
> index 000..37ea2d1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/frame-header-2.c
> @@ -0,0 +1,21 @@
> +/* Verify that we do optimize away the frame header in foo when using
> +   -mframe-header-opt by checking the stack pointer increment done in
> +   that function.  Without the optimization foo should increment the
> +   stack by 32 bytes, with the optimization it would only be 8 bytes.
> +*/
> +
> +/* { dg-do compile } */
> +/* { dg-options "-mframe-header-opt" } */
> +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> +/* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-8" } } */
> +
> +void __attribute__((noinline))
> +bar (int* a)
> +{
> +  *a = 1;
> +}
> +
> +void
> +foo (int a)
> +{
> +  bar (&a);
> +}
> diff --git a/gcc/testsuite/gcc.target/mips/frame-header-3.c
> b/gcc/testsuite/gcc.target/mips/frame-header-3.c
> new file mode 100644
> index 000..1cb1547
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/frame-header-3.c
> @@ -0,0 +1,22 @@
> +/* Verify that we do not optimize away the frame header in foo when using
> +   -mframe-header-opt but are calling a weak function that may be
> overridden
> +   by a different function that does need the frame header.  Without the
> +   optimization foo should increment the stack by 32 bytes, with the
> +   optimization it would only be 8 bytes.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-mframe-header-opt" } */
> +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> +/* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-32" } } */
> +
> +void __attribute__((noinline, weak))
> +bar (int* a)
> +{
> +  *a = 1;
> +}
> +
> +void
> +foo (int a)
> +{
> +  bar (&a);
> +}
> diff --git a/gcc/testsuite/gcc.target/mips/mips.exp
> b/gcc/testsuite/gcc.target/mips/mips.exp
> index 42e7fff..0f2d6a2 100644
> --- a/gcc/testsuite/gcc.target/mips/mips.exp
> +++ b/gcc/testsuite/gcc.target/mips/mips.exp
> @@ -256,6 +256,7 @@ set mips_option_groups {
>  maddps "HAS_MADDPS"
>  lsa "(|!)HAS_LSA"
>  section_start "-Wl,--section-start=.*"
> +frame-header "-mframe-header-opt|-mno-frame-header-opt"
>  }
> 
>  for { set option 0 } { $option < 32 } { incr option } {
> 



RE: [PATCH, MIPS] Frame header optimization for MIPS O32 ABI

2015-10-06 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Tuesday, October 06, 2015 4:39 AM
> To: Moore, Catherine; Steve Ellcey
> Cc: GCC Patches
> Subject: RE: [PATCH, MIPS] Frame header optimization for MIPS O32 ABI
> 
> Moore, Catherine  writes:
> > The patch itself looks good, but the tests that you added need a little more
> work.
> >
> > I tested with the mips-sde-elf-lite configuration and I'm seeing
> > failures for many options.  The main failure mode seems to be that the
> > stack is incremented by 24 instead of 32.
> > I tried this change in frame-header-1.c and frame-header-3.c:
> >
> > /* { dg-final { scan-assembler-not "\taddiu\t\\\$sp,\\\$sp,-8" } }
> >
> > instead of:
> >
> > /* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-32" }
> 
> I'd quite like to be specific about the frame layout we expect as the testcase
> is so simple that I think it should be predictable over time. Did you happen 
> to
> see a pattern to the failure? i.e. Could it be non-o32 ABIs? I'm not a fan of
> scan-assembler-nots in general as it is so easy to change exact output text
> and never match them anyway even if the offending instruction is
> generated.
There was not a pattern to the failure and I witnessed it for o32 ABIs.
> 
> > There are still errors in frame-header-2.c when compiling with -mips16
> > and -mabi=64 (this one uses a daddiu).
> > Also, the tests fail for -flto variants.
> 
> Let's just add NOMIPS16 (I think that is the macro) to the functions and lock
> the tests down to -mabi=32 which is the only ABI they are valid for anyway.
> 
> Matthew
> 
> > Would you please fix this up and resubmit?
> >
> > Thanks,
> > Catherine
> >
> > >
> > >
> > > diff --git a/gcc/testsuite/gcc.target/mips/frame-header-1.c
> > > b/gcc/testsuite/gcc.target/mips/frame-header-1.c
> > > new file mode 100644
> > > index 000..8495e0f
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/mips/frame-header-1.c
> > > @@ -0,0 +1,21 @@
> > > +/* Verify that we do not optimize away the frame header in foo when
> using
> > > +   -mno-frame-header-opt by checking the stack pointer increment done
> in
> > > +   that function.  Without the optimization foo should increment the
> stack
> > > +   by 32 bytes, with the optimization it would only be 8 bytes.  */
> > > +
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-mno-frame-header-opt" } */
> > > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> > > +/* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-32" } } */
> > > +
> > > +void __attribute__((noinline))
> > > +bar (int* a)
> > > +{
> > > +  *a = 1;
> > > +}
> > > +
> > > +void
> > > +foo (int a)
> > > +{
> > > +  bar (&a);
> > > +}
> > > diff --git a/gcc/testsuite/gcc.target/mips/frame-header-2.c
> > > b/gcc/testsuite/gcc.target/mips/frame-header-2.c
> > > new file mode 100644
> > > index 000..37ea2d1
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/mips/frame-header-2.c
> > > @@ -0,0 +1,21 @@
> > > +/* Verify that we do optimize away the frame header in foo when using
> > > +   -mframe-header-opt by checking the stack pointer increment done in
> > > +   that function.  Without the optimization foo should increment the
> > > +   stack by 32 bytes, with the optimization it would only be 8 bytes.
> > > +*/
> > > +
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-mframe-header-opt" } */
> > > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> > > +/* { dg-final { scan-assembler "\taddiu\t\\\$sp,\\\$sp,-8" } } */
> > > +
> > > +void __attribute__((noinline))
> > > +bar (int* a)
> > > +{
> > > +  *a = 1;
> > > +}
> > > +
> > > +void
> > > +foo (int a)
> > > +{
> > > +  bar (&a);
> > > +}
> > > diff --git a/gcc/testsuite/gcc.target/mips/frame-header-3.c
> > > b/gcc/testsuite/gcc.target/mips/frame-header-3.c
> > > new file mode 100644
> > > index 000..1cb1547
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/mips/frame-header-3.c
> > > @@ -0,0 +1,22 @@
> > > +/* Verify t

RE: [PATCH, MIPS] Frame header optimization for MIPS O32 ABI

2015-10-07 Thread Moore, Catherine


> -Original Message-
> From: Steve Ellcey [mailto:sell...@imgtec.com]
> Sent: Tuesday, October 06, 2015 6:03 PM
> To: Moore, Catherine
> Cc: Matthew Fortune; GCC Patches
> Subject: RE: [PATCH, MIPS] Frame header optimization for MIPS O32 ABI
> 
> On Tue, 2015-10-06 at 12:02 +, Moore, Catherine wrote:
> 
> > > Moore, Catherine  writes:
> > > > The patch itself looks good, but the tests that you added need a little
> more
> > > work.
> 
> Here is a new patch for just the tests.  I added NOMIPS16 and the
> -mabi=32 flag as Matthew suggested and I also added -mno-abicalls.
> 
> Without the -mno-abicalls the MIPS linux compiler defaulted to
> -mabicalls and allocated 32 bytes without the optimization and the MIPS
> elf compiler defaulted to -mno-abicalls and allocated 24 bytes without
> the optimization.  This synchronizes them so they both allocate 24 bytes
> without the optimization and 8 bytes with it.  Everything should pass
> now, it passed for me using mips-mti-linux-gnu and mips-mti-elf.
> 
> Steve Ellcey
> sell...@imgtec.com
> 
> 
> 2015-10-06  Steve Ellcey  
> 
>   * gcc.target/mips/mips.exp (mips_option_groups): Add -mframe-
> header-opt
>   and -mno-frame-header-opt options.
>   * gcc.target/mips/frame-header-1.c: New file.
>   * gcc.target/mips/frame-header-2.c: New file.
>   * gcc.target/mips/frame-header-3.c: New file.
> 
> 
This looks good to go now.
Thanks,
Catherine



RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic architecture

2015-10-22 Thread Moore, Catherine


> -Original Message-
> From: Robert Suchanek [mailto:robert.sucha...@imgtec.com]
> Sent: Friday, September 04, 2015 10:21 AM
> To: Matthew Fortune; Richard Sandiford
> Cc: Moore, Catherine; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Disable -mbranch-likely for -Os when targetting generic
> architecture
> 
> Hi,
> 
> > Richard Sandiford  writes:
> > > Robert Suchanek  writes:
> > > > The patch below disables generation of the branch likely
> > > > instructions for -
> > Os
> > > > but only for generic architecture.  The branch likely may result
> > > > in some code size reduction but the cost of running the code on R6
> > > > core is
> > significant.
> > >
> > > How about instead splitting PTF_AVOID_BRANCHLIKELY into
> > > PTF_AVOID_BRANCHLIKELY_SPEED and
> PTF_AVOID_BRANCHLIKELY_SIZE?
> > > We could have PTF_AVOID_BRANCHLIKELY_ALWAYS as an OR of the two.
> >
> > This sounds OK and is nicer.
> >
> > > Anything that does string ops on the architecture is suspicious :-)
> >
> > You can blame me for this.  I advocated the string comparison approach
> > as I had to do the same thing in gas IIRC for some feature and
> > couldn't think of anything better to suggest.
> 
> Here is an updated version that use the suggested method. Ok to apply?
> 
> Regards,
> Robert
> 
> 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.




RE: [PATCH, Mips] Compact branch/delay slot optimization.

2015-10-28 Thread Moore, Catherine


> -Original Message-
> From: Simon Dardis [mailto:simon.dar...@imgtec.com]
> Sent: Tuesday, October 06, 2015 10:00 AM
> To: Moore, Catherine; Matthew Fortune
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH, Mips] Compact branch/delay slot optimization.
> 
> Hello,
> 
> I'd like to resubmit the previous patch as it failed to check if the branch 
> inside
> the sequence had a compact form.
> 
> Thanks,
> Simon
> 
> gcc/
> * config/mips/mips.c: (mips_breakable_sequence_p): New function.
>   (mips_break_sequence): New function.
>   (mips_reorg_process_insns) Use them. Use compact branches in
> selected
>   situations.
> 
> gcc/testsuite/
> * gcc.target/mips/split-ds-sequence.c: Test for the above.

Hi Simon,
This patch looks okay with the exception of one stylistic change.
Please change all instances of :
+mips_breakable_sequence_p (rtx_insn * insn)
To:
+mips_breakable_sequence_p (rtx_insn *insn)
Okay, with those changes.
Thanks,
Catherine


> 
> Index: config/mips/mips.c
> ==
> =
> --- config/mips/mips.c(revision 228282)
> +++ config/mips/mips.c(working copy)
> @@ -16973,6 +16973,34 @@
>}
>  }
> 
> +/* A SEQUENCE is breakable iff the branch inside it has a compact form
> +   and the target has compact branches.  */
> +
> +static bool
> +mips_breakable_sequence_p (rtx_insn * insn)
> +{
> +  return (insn && GET_CODE (PATTERN (insn)) == SEQUENCE
> +   && TARGET_CB_MAYBE
> +   && get_attr_compact_form (SEQ_BEGIN (insn)) !=
> COMPACT_FORM_NEVER);
> +}
> +
> +/* Remove a SEQUENCE and replace it with the delay slot instruction
> +   followed by the branch and return the instruction in the delay slot.
> +   Return the first of the two new instructions.
> +   Subroutine of mips_reorg_process_insns.  */
> +
> +static rtx_insn *
> +mips_break_sequence (rtx_insn * insn)
> +{
> +  rtx_insn * before = PREV_INSN (insn);
> +  rtx_insn * branch = SEQ_BEGIN (insn);
> +  rtx_insn * ds = SEQ_END (insn);
> +  remove_insn (insn);
> +  add_insn_after (ds, before, NULL);
> +  add_insn_after (branch, ds, NULL);
> +  return ds;
> +}
> +
>  /* Go through the instruction stream and insert nops where necessary.
> Also delete any high-part relocations whose partnering low parts
> are now all dead.  See if the whole function can then be put into
> @@ -17065,6 +17093,68 @@
>   {
> if (GET_CODE (PATTERN (insn)) == SEQUENCE)
>   {
> +   rtx_insn * next_active = next_active_insn (insn);
> +   /* Undo delay slots to avoid bubbles if the next instruction can
> +  be placed in a forbidden slot or the cost of adding an
> +  explicit NOP in a forbidden slot is OK and if the SEQUENCE is
> +  safely breakable.  */
> +   if (TARGET_CB_MAYBE
> +   && mips_breakable_sequence_p (insn)
> +   && INSN_P (SEQ_BEGIN (insn))
> +   && INSN_P (SEQ_END (insn))
> +   && ((next_active
> +&& INSN_P (next_active)
> +&& GET_CODE (PATTERN (next_active)) != SEQUENCE
> +&& get_attr_can_delay (next_active) ==
> CAN_DELAY_YES)
> +   || !optimize_size))
> + {
> +   /* To hide a potential pipeline bubble, if we scan backwards
> +  from the current SEQUENCE and find that there is a load
> +  of a value that is used in the CTI and there are no
> +  dependencies between the CTI and instruction in the
> delay
> +  slot, break the sequence so the load delay is hidden.  */
> +   HARD_REG_SET uses;
> +   CLEAR_HARD_REG_SET (uses);
> +   note_uses (&PATTERN (SEQ_BEGIN (insn)),
> record_hard_reg_uses,
> +  &uses);
> +   HARD_REG_SET delay_sets;
> +   CLEAR_HARD_REG_SET (delay_sets);
> +   note_stores (PATTERN (SEQ_END (insn)),
> record_hard_reg_sets,
> +&delay_sets);
> +
> +   rtx prev = prev_active_insn (insn);
> +   if (prev
> +   && GET_CODE (PATTERN (prev)) == SET
> +   && MEM_P (SET_SRC (PATTERN (prev
> + {
> +   HARD_REG_SET sets;
> +   CLEAR_HARD_REG_SET (sets);
> +   note_stores (PATTERN (prev), record_hard_reg_sets

[Committed] Testsuite update for MIPS16

2015-10-28 Thread Moore, Catherine
I've now committed this patch to fix a couple of target-specific failures for 
MIP16 multilibs.

2015-10-28  Catherine Moore  

* gcc.target/mips/oddspreg-3.c: Disable for MIPS16.
* gcc.target/mips/oddspreg-6.c: Likewise.
* gcc.target/mips/oddspreg-1.c: Likewise.
* gcc.target/mips/oddspreg-2.c: Likewise.


Index: gcc.target/mips/oddspreg-6.c
===
--- gcc.target/mips/oddspreg-6.c(revision 229495)
+++ gcc.target/mips/oddspreg-6.c(working copy)
@@ -2,7 +2,7 @@
 /* { dg-skip-if "needs asm output" { *-*-* } { "-fno-fat-lto-objects" } { "" } 
} */
 /* { dg-options "-mabi=32 -mfpxx -mhard-float" } */

-void
+NOMIPS16 void
 foo ()
 {
   register float foo asm ("$f1"); /* { dg-error "isn't suitable for" } */
Index: gcc.target/mips/oddspreg-1.c
===
--- gcc.target/mips/oddspreg-1.c(revision 229495)
+++ gcc.target/mips/oddspreg-1.c(working copy)
@@ -5,7 +5,7 @@
 #error "Incorrect number of single-precision registers reported"
 #endif

-void
+NOMIPS16 void
 foo ()
 {
   register float foo asm ("$f1");
Index: gcc.target/mips/oddspreg-2.c
===
--- gcc.target/mips/oddspreg-2.c(revision 229495)
+++ gcc.target/mips/oddspreg-2.c(working copy)
@@ -2,7 +2,7 @@
 /* { dg-skip-if "needs asm output" { *-*-* } { "-fno-fat-lto-objects" } { "" } 
} */
 /* { dg-options "-mabi=32 -mno-odd-spreg -mhard-float" } */

-void
+NOMIPS16 void
 foo ()
 {
   register float foo asm ("$f1"); /* { dg-error "isn't suitable for" } */
Index: gcc.target/mips/oddspreg-3.c
===
--- gcc.target/mips/oddspreg-3.c(revision 229495)
+++ gcc.target/mips/oddspreg-3.c(working copy)
@@ -2,7 +2,7 @@
 /* { dg-skip-if "needs asm output" { *-*-* } { "-fno-fat-lto-objects" } { "" } 
} */
 /* { dg-options "-mabi=32 -mfp32 -march=loongson3a -mhard-float" } */

-void
+NOMIPS16 void
 foo ()
 {
   register float foo asm ("$f1"); /* { dg-error "isn't suitable for" } */



RE: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.

2015-11-03 Thread Moore, Catherine


> -Original Message-
> From: Simon Dardis [mailto:simon.dar...@imgtec.com]
> Sent: Wednesday, October 07, 2015 6:51 AM
> To: Alan Lawrence; Matthew Fortune; Moore, Catherine
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH, MIPS, PR/61114] Migrate to reduc_..._scal optabs.
> 
> On the change from smin/smax it was a deliberate change as I managed to
> confuse myself of the mode patterns, correct version follows. Reverted back
> to VWHB for smax/smin. Stylistic point addressed.
> 
> No new regression, ok for commit?
> 

Yes, OK to commit.  Sorry for the delay in review.
Catherine

> 
> Index: config/mips/loongson.md
> ==
> =
> --- config/mips/loongson.md   (revision 228282)
> +++ config/mips/loongson.md   (working copy)
> @@ -852,58 +852,66 @@
>"dsrl\t%0,%1,%2"
>[(set_attr "type" "fcvt")])
> 
> -(define_expand "reduc_uplus_"
> -  [(match_operand:VWH 0 "register_operand" "")
> -   (match_operand:VWH 1 "register_operand" "")]
> +(define_insn "vec_loongson_extract_lo_"
> +  [(set (match_operand: 0 "register_operand" "=r")
> +(vec_select:
> +  (match_operand:VWHB 1 "register_operand" "f")
> +  (parallel [(const_int 0)])))]
>"TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
> -{
> -  mips_expand_vec_reduc (operands[0], operands[1], gen_add3);
> -  DONE;
> -})
> +  "mfc1\t%0,%1"
> +  [(set_attr "type" "mfc")])
> 
> -; ??? Given that we're not describing a widening reduction, we should
> -; not have separate optabs for signed and unsigned.
> -(define_expand "reduc_splus_"
> -  [(match_operand:VWHB 0 "register_operand" "")
> +(define_expand "reduc_plus_scal_"
> +  [(match_operand: 0 "register_operand" "")
> (match_operand:VWHB 1 "register_operand" "")]
>"TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>  {
> -  emit_insn (gen_reduc_uplus_(operands[0], operands[1]));
> +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> +  mips_expand_vec_reduc (tmp, operands[1], gen_add3);
> +  emit_insn (gen_vec_loongson_extract_lo_ (operands[0], tmp));
>DONE;
>  })
> 
> -(define_expand "reduc_smax_"
> -  [(match_operand:VWHB 0 "register_operand" "")
> +(define_expand "reduc_smax_scal_"
> +  [(match_operand: 0 "register_operand" "")
> (match_operand:VWHB 1 "register_operand" "")]
>"TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>  {
> -  mips_expand_vec_reduc (operands[0], operands[1], gen_smax3);
> +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> +  mips_expand_vec_reduc (tmp, operands[1], gen_smax3);
> +  emit_insn (gen_vec_loongson_extract_lo_ (operands[0], tmp));
>DONE;
>  })
> 
> -(define_expand "reduc_smin_"
> -  [(match_operand:VWHB 0 "register_operand" "")
> +(define_expand "reduc_smin_scal_"
> +  [(match_operand: 0 "register_operand" "")
> (match_operand:VWHB 1 "register_operand" "")]
>"TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>  {
> -  mips_expand_vec_reduc (operands[0], operands[1], gen_smin3);
> +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> +  mips_expand_vec_reduc (tmp, operands[1], gen_smin3);
> +  emit_insn (gen_vec_loongson_extract_lo_ (operands[0], tmp));
>DONE;
>  })
> 
> -(define_expand "reduc_umax_"
> -  [(match_operand:VB 0 "register_operand" "")
> +(define_expand "reduc_umax_scal_"
> +  [(match_operand: 0 "register_operand" "")
> (match_operand:VB 1 "register_operand" "")]
>"TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>  {
> -  mips_expand_vec_reduc (operands[0], operands[1],
> gen_umax3);
> +  rtx tmp = gen_reg_rtx (GET_MODE (operands[1]));
> +  mips_expand_vec_reduc (tmp, operands[1], gen_umax3);
> +  emit_insn (gen_vec_loongson_extract_lo_ (operands[0], tmp));
>DONE;
>  })
> 
> -(define_expand "reduc_umin_"
> -  [(match_operand:VB 0 "register_operand" "")
> +(define_expand "reduc_umin_scal_"
> +  [(match_operand: 0 "register_operand" "")
> (match_operand:VB 1 "register_operand" "")]
>"TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>  {
&

RE: [Patch, MIPS] Frame header optimization for MIPS (part 2)

2015-11-24 Thread Moore, Catherine


> -Original Message-
> From: Steve Ellcey [mailto:sell...@imgtec.com]
> Sent: Friday, October 23, 2015 2:08 PM
> To: Myers, Joseph
> Cc: Bernd Schmidt; Mike Stump; gcc-patches@gcc.gnu.org;
> matthew.fort...@imgtec.com; Moore, Catherine
> Subject: Re: [Patch, MIPS] Frame header optimization for MIPS (part 2)
> 
> Just to follow up on this string, here is a new version of the patch
> with the extraneous parenthesis removed.
> 
> Steve Ellcey
> sell...@imgtec.com
> 
> 
> 2015-10-23  Steve Ellcey  
> 
>   * frame-header-opt.c (gate): Check for optimize > 0.
>   (has_inlined_assembly): New function.
>   (needs_frame_header_p): Remove is_leaf_function check,
>   add argument type check.
>   (callees_functions_use_frame_header): Add is_leaf_function
>   and has_inlined_assembly calls..
>   (set_callers_may_not_allocate_frame): New function.
>   (frame_header_opt): Add is_leaf_function call, add
>   set_callers_may_not_allocate_frame call.
>   * config/mips/mips.c (mips_compute_frame_info): Add check
>   to see if callee saved regs can be put in frame header.
>   (mips_expand_prologue): Add check to see if step1 is zero,
>   fix cfa restores when using frame header to store regs.
>   (mips_can_use_return_insn): Check to see if registers are
>   stored in frame header.
>   * config/mips/mips.h (machine_function): Add
>   callers_may_not_allocate_frame and
>   use_frame_header_for_callee_saved_regs fields.

This is okay after making the minor corrections embedded below.
I'm sorry that it took me so long to review this patch.
Catherine

> 
> diff --git a/gcc/config/mips/frame-header-opt.c b/gcc/config/mips/frame-
> header-opt.c
> index 7c7b1f2..2cf589d 100644
> --- a/gcc/config/mips/frame-header-opt.c
> +++ b/gcc/config/mips/frame-header-opt.c
> @@ -125,6 +125,29 @@ is_leaf_function (function *fn)
>return true;
>  }
> 
> +/* Return true if this function has inline assembly code or if we cannot
> +   be certain that it does not.  False if know that there is no inline
> +   assembly.  */
> +

 s/False if know/False if we know/

> @@ -136,20 +159,26 @@ needs_frame_header_p (function *fn)
>if (fn->decl == NULL)
>  return true;
> 
> -  if (fn->stdarg || !is_leaf_function (fn))
> +  if (fn->stdarg)
>  return true;
> 
>for (t = DECL_ARGUMENTS (fn->decl); t; t = TREE_CHAIN (t))
>  {
>if (!use_register_for_decl (t))
> -   return true;
> + return true;
> +
> +  /* Some 64 bit types may get copied to general registers using the 
> frame
> +  header, see mips_output_64bit_xfer.  Checking for SImode only may be
> + overly restrictive but it is gauranteed to be safe. */

  s/64 bit/64-bit/
 s/gauranteed/guaranteed/

> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index c5affc8..5a9d48d 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -10465,6 +10465,35 @@ mips_compute_frame_info (void)
>frame->cop0_sp_offset = offset - UNITS_PER_WORD;
>  }
> 
> +  /* Determine if we can save the callee saved registers in the frame
> + header.  Restrict this to functions where there is no other reason
> + to allocate stack space so that we can completely eliminate the
> + instructions that modify the stack pointer.  */
> +
  s/callee saved/callee-saved/
  s/completely//

> 
> 
> 
> 2015-10-23  Steve Ellcey  
> 
>   * gcc.target/mips/frame-header-4.c: New test.
> 
> diff --git a/gcc/testsuite/gcc.target/mips/frame-header-4.c
> b/gcc/testsuite/gcc.target/mips/frame-header-4.c
> index e69de29..3cddba1 100644
> --- a/gcc/testsuite/gcc.target/mips/frame-header-4.c
> +++ b/gcc/testsuite/gcc.target/mips/frame-header-4.c
> @@ -0,0 +1,20 @@
> +/* Verify that we can optimize away the frame header allocation in bar
> +   by having it use its frame header to store $31 in before calling foo.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-mframe-header-opt -mabi=32" } */
> +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> +/* { dg-final { scan-assembler-not "\taddiu\t\\\$sp" } } */
> +
> +int __attribute__ ((noinline))
> +foo (int a, int b)
> +{
> + return a + b;
> +}
> +
> +int  __attribute__ ((noinline))
> +bar (int a, int b)
> +{
> + return 1 + foo(a,b);
> +}
> +

  Space after foo, please.  Change the two tabs to two space each.



RE: [RFA] Compact EH Patch

2015-11-25 Thread Moore, Catherine


> -Original Message-
> From: Richard Henderson [mailto:r...@redhat.com]
> Sent: Friday, September 18, 2015 3:25 PM
> To: Moore, Catherine; gcc-patches@gcc.gnu.org
> Cc: ja...@redhat.com; Matthew Fortune
> Subject: Re: [RFA] Compact EH Patch
> 
> > Index: libgcc/libgcc-std.ver.in
> >
> ==
> =
> > --- libgcc/libgcc-std.ver.in(revision 226409)
> > +++ libgcc/libgcc-std.ver.in(working copy)
> > @@ -1918,6 +1918,7 @@ GCC_4.6.0 {
> >__morestack_current_segment
> >__morestack_initial_sp
> >__splitstack_find
> > +  _Unwind_GetEhEncoding
> >  }
> >
> >  %inherit GCC_4.7.0 GCC_4.6.0
> > @@ -1938,3 +1939,8 @@ GCC_4.7.0 {
> >  %inherit GCC_4.8.0 GCC_4.7.0
> >  GCC_4.8.0 {
> >  }
> > +
> > +%inherit GCC_4.8.0 GCC_4.7.0
> > +GCC_4.8.0 {
> > +  __register_frame_info_header_bases
> > +}
> 
> You can't push new symbols into old versions.  These have to go into the
> version for the current gcc.
> 
> > Index: libstdc++-v3/config/abi/pre/gnu.ver
> >
> ==
> =
> > --- libstdc++-v3/config/abi/pre/gnu.ver (revision 226409)
> > +++ libstdc++-v3/config/abi/pre/gnu.ver (working copy)
> > @@ -1909,6 +1909,7 @@ CXXABI_1.3 {
> >  __gxx_personality_v0;
> >  __gxx_personality_sj0;
> >  __gxx_personality_seh0;
> > +__gnu_compact_pr2;
> >  __dynamic_cast;
> >
> >  # *_type_info classes, ctor and dtor
> > Index: libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
> >
> ==
> =
> > --- libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
>   (revision 226409)
> > +++ libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
>   (working copy)
> > @@ -200,6 +200,7 @@ CXXABI_2.0 {
> >  __cxa_vec_new;
> >  __gxx_personality_v0;
> >  __gxx_personality_sj0;
> > +__gnu_compact_pr2;
> >  __dynamic_cast;
> >
> >  # std::exception_ptr
> 
> Likewise.
> 
I'm getting ready to post the updates to this patch -- hopefully, I can still 
get it in GCC 6.0.
I'm not sure how to tell what the current CXXABI is for these two files.  
Should it be CXXABI_2.0 for both of these?
Thanks,
Catherine


RE: [PATCH] MIPS/GCC/doc: Reorder `-mcompact-branches='

2015-11-26 Thread Moore, Catherine


> -Original Message-
> From: Maciej W. Rozycki [mailto:ma...@imgtec.com]
> Sent: Thursday, November 26, 2015 9:01 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Moore, Catherine; Matthew Fortune
> Subject: [PATCH] MIPS/GCC/doc: Reorder `-mcompact-branches='
> 
> Move the `-mcompact-branches=' option out of the middle of a block of
> floating-point options.  The option is not related to FP in any way.
> Place it immediately below other branch instruction selection options.
> 
>   gcc/
>   * doc/invoke.texi (Option Summary) : Reorder
>   `-mcompact-branches='.
>   (MIPS Options): Likewise.
> ---
> 
>  OK to apply?
> 
 Yes -- thanks.


RE: [RFA] Compact EH Patch

2015-12-01 Thread Moore, Catherine
Ping?

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Moore, Catherine
> Sent: Wednesday, November 25, 2015 11:58 AM
> To: Richard Henderson; gcc-patches@gcc.gnu.org
> Cc: ja...@redhat.com; Matthew Fortune
> Subject: RE: [RFA] Compact EH Patch
> 
> 
> 
> > -Original Message-
> > From: Richard Henderson [mailto:r...@redhat.com]
> > Sent: Friday, September 18, 2015 3:25 PM
> > To: Moore, Catherine; gcc-patches@gcc.gnu.org
> > Cc: ja...@redhat.com; Matthew Fortune
> > Subject: Re: [RFA] Compact EH Patch
> >
> > > Index: libgcc/libgcc-std.ver.in
> > >
> >
> ==
> > =
> > > --- libgcc/libgcc-std.ver.in  (revision 226409)
> > > +++ libgcc/libgcc-std.ver.in  (working copy)
> > > @@ -1918,6 +1918,7 @@ GCC_4.6.0 {
> > >__morestack_current_segment
> > >__morestack_initial_sp
> > >__splitstack_find
> > > +  _Unwind_GetEhEncoding
> > >  }
> > >
> > >  %inherit GCC_4.7.0 GCC_4.6.0
> > > @@ -1938,3 +1939,8 @@ GCC_4.7.0 {
> > >  %inherit GCC_4.8.0 GCC_4.7.0
> > >  GCC_4.8.0 {
> > >  }
> > > +
> > > +%inherit GCC_4.8.0 GCC_4.7.0
> > > +GCC_4.8.0 {
> > > +  __register_frame_info_header_bases
> > > +}
> >
> > You can't push new symbols into old versions.  These have to go into
> > the version for the current gcc.
> >
> > > Index: libstdc++-v3/config/abi/pre/gnu.ver
> > >
> >
> ==
> > =
> > > --- libstdc++-v3/config/abi/pre/gnu.ver   (revision 226409)
> > > +++ libstdc++-v3/config/abi/pre/gnu.ver   (working copy)
> > > @@ -1909,6 +1909,7 @@ CXXABI_1.3 {
> > >  __gxx_personality_v0;
> > >  __gxx_personality_sj0;
> > >  __gxx_personality_seh0;
> > > +__gnu_compact_pr2;
> > >  __dynamic_cast;
> > >
> > >  # *_type_info classes, ctor and dtor
> > > Index: libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
> > >
> >
> ==
> > =
> > > --- libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
> > (revision 226409)
> > > +++ libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
> > (working copy)
> > > @@ -200,6 +200,7 @@ CXXABI_2.0 {
> > >  __cxa_vec_new;
> > >  __gxx_personality_v0;
> > >  __gxx_personality_sj0;
> > > +__gnu_compact_pr2;
> > >  __dynamic_cast;
> > >
> > >  # std::exception_ptr
> >
> > Likewise.
> >
> I'm getting ready to post the updates to this patch -- hopefully, I can still 
> get it
> in GCC 6.0.
> I'm not sure how to tell what the current CXXABI is for these two files.
> Should it be CXXABI_2.0 for both of these?
> Thanks,
> Catherine


RE: [mips] Rotate stack checking loop

2015-12-02 Thread Moore, Catherine


> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Eric Botcazou
> Sent: Thursday, November 12, 2015 4:51 PM
> To: gcc-patches@gcc.gnu.org
> Subject: [mips] Rotate stack checking loop
> 
> Hi,
> 
> this patch rotates the loop generated in the prologue to do stack checking
> when -fstack-check is specified, thereby saving one branch instruction.  It
> was initially implemented as a WHILE loop to match the generic
> implementation but can be turned into a DO-WHILE loop because the
> amount of stack to be checked is known at compile time (since it's the static
> part of the frame).
> 
> Unfortunately I don't have access to MIPS hardware any more so I only
> verified
> that the assembly code is as expected and can be assembled.   OK for
> mainline?
> 
> 
> 2015-11-12  Eric Botcazou  
> 
>   * config/mips/mips.c (mips_emit_probe_stack_range): Adjust.
>   (mips_output_probe_stack_range): Rotate the loop and simplify.
> 
This is OK.


RE: [Patch][gcn, nvptx, offloading] mkoffload – handle -fpic/-fPIC

2020-07-12 Thread Moore, Catherine


>-Original Message-
>From: Gcc-patches [mailto:gcc-patches-boun...@gcc.gnu.org] On Behalf
>Of Tom de Vries
>Sent: Friday, July 3, 2020 6:52 AM
>To: Moore, Catherine ; Burnus, Tobias
>; gcc-patches ;
>Jakub Jelinek 
>Cc: Schwinge, Thomas ; Stubbs,
>Andrew 
>Subject: Re: [Patch][gcn, nvptx, offloading] mkoffload – handle -fpic/-fPIC
>
>On 6/26/20 9:35 PM, Moore, Catherine wrote:
>> Hi Tom,
>>
>> It doesn't look like you were explicitly cc'd on this patch and probably
>haven't seen it.  Would you mind taking a look and approving the nvptx
>portions?
>>
>
>[ thanks for the ping, I think was explicitly cc-ed though.  I'm usually
>not too fast in reviewing, and this time a vacation added to that. ]
>
>The patch looks good to me.

Thanks for the review.
Catherine

>
>I tried out the patch with one test-case and -pie -fPIC/-fpic already
>seems to works, so perhaps we could have at least one test-case
>exercising this in libgomp?  That sounds easier to do than the
>shared-lib test-case.
>
>I think it's a good idea though to do fPIE/fpie as well, either in this
>patch or as follow-up.
>
>Thanks,
>- Tom
>
>> Thanks,
>> Catherine
>>
>>> -Original Message-
>>> From: Gcc-patches [mailto:gcc-patches-boun...@gcc.gnu.org] On
>Behalf
>>> Of Burnus, Tobias
>>> Sent: Tuesday, June 23, 2020 11:21 AM
>>> To: gcc-patches ; Jakub Jelinek
>>> 
>>> Cc: Stubbs, Andrew ; Schwinge,
>Thomas
>>> 
>>> Subject: [Patch][gcn, nvptx, offloading] mkoffload – handle -fpic/-fPIC
>>>
>>> If the offloading code is (only) in a library, one can come up
>>> with the idea to build those parts as shared library – and link
>>> it to the nonoffloading code.(*)
>>>
>>> Currently, this fails as the mkoffload calls the nonoffloading
>>> compiler without the -fpic/-fPIC flags, even though the compiler
>>> was originally invoked with those options. – And at some point,
>>> the linker then complains.
>>>
>>> This patch simply adds -fpic/-fPIC to the calls to the nonoffloading
>>> ("host") compiler, invoked from mkoffload, if they were present before.
>>>
>>> For the testcase at hand, this works with both AMDGCN and nvptx
>>> with the attached patch.
>>>
>>> OK for the trunk?
>>>
>>> Tobias
>>>
>>> PS: I think as mid-/longterm project it would be nice to test this
>>> in the testsuite, but that's unfortunately a larger task.
>>>
>>> (*) Thomas mentioned that this is supposed to work also in more
>>> complex cases than the one I outlined, although, that is probably
>>> currently the most common one.
>>>
>>> -
>>> Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634
>München /
>>> Germany
>>> Registergericht München HRB 106955, Geschäftsführer: Thomas
>Heurung,
>>> Alexander Walter


RE: [Patch][gcn, nvptx, offloading] mkoffload – handle -fpic/-fPIC

2020-06-26 Thread Moore, Catherine
Hi Tom,

It doesn't look like you were explicitly cc'd on this patch and probably 
haven't seen it.  Would you mind taking a look and approving the nvptx portions?

Thanks,
Catherine

>-Original Message-
>From: Gcc-patches [mailto:gcc-patches-boun...@gcc.gnu.org] On Behalf
>Of Burnus, Tobias
>Sent: Tuesday, June 23, 2020 11:21 AM
>To: gcc-patches ; Jakub Jelinek
>
>Cc: Stubbs, Andrew ; Schwinge, Thomas
>
>Subject: [Patch][gcn, nvptx, offloading] mkoffload – handle -fpic/-fPIC
>
>If the offloading code is (only) in a library, one can come up
>with the idea to build those parts as shared library – and link
>it to the nonoffloading code.(*)
>
>Currently, this fails as the mkoffload calls the nonoffloading
>compiler without the -fpic/-fPIC flags, even though the compiler
>was originally invoked with those options. – And at some point,
>the linker then complains.
>
>This patch simply adds -fpic/-fPIC to the calls to the nonoffloading
>("host") compiler, invoked from mkoffload, if they were present before.
>
>For the testcase at hand, this works with both AMDGCN and nvptx
>with the attached patch.
>
>OK for the trunk?
>
>Tobias
>
>PS: I think as mid-/longterm project it would be nice to test this
>in the testsuite, but that's unfortunately a larger task.
>
>(*) Thomas mentioned that this is supposed to work also in more
>complex cases than the one I outlined, although, that is probably
>currently the most common one.
>
>-
>Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München /
>Germany
>Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung,
>Alexander Walter


RE: [PATCH] Fix mips hang with --help=target --help=optimizers (PR target/82880)

2017-11-21 Thread Moore, Catherine


> -Original Message-
> From: Jakub Jelinek [mailto:ja...@redhat.com]
> Sent: Tuesday, November 21, 2017 9:07 AM
> To: Moore, Catherine ; Matthew
> Fortune 
> Cc: gcc-patches@gcc.gnu.org; James Cowgill
> 
> Subject: [PATCH] Fix mips hang with --help=target --help=optimizers (PR
> target/82880)
> 
> Hi!
> 
> This is a patch from James that has been sitting in bugzilla for a few
> weeks.  The bug is that mips_register_frame_header_opt registers
> the pass from a static variable which has an automatic variable in the
> initializer, which means that the first time it is called it is registered
> properly, but if it is called multiple times (e.g. possible with gccjit
> or multiple --help options), then the second time it creates a new pass,
> but registers with the old pass (since the var isn't initialized again).
> register_pass doesn't store the address it is called with anywhere, just
> uses the fields of the struct and stores the pass (first field).
> All other spots that call register_pass in all backends do it properly
> it seems.
> 
> I've just fixed up formatting and added a testcase, tested with cross to
> mips and tested the testcase on x86_64-linux and i686-linux.
> 
> Ok for trunk?
> 
> 2017-11-21  James Cowgill  
>   Jakub Jelinek  
> 
>   PR target/82880
>   * config/mips/frame-header-opt.c
> (mips_register_frame_header_opt):
>   Remove static keyword from f variable.
> 
>   * gcc.dg/opts-8.c: New test.
> 
Yes, this is OK.  Thanks for fixing.


[Committed] Remove myself as MIPS maintainer

2018-04-25 Thread Moore, Catherine
2018-04-25  Catherine Moore 

* MAINTAINERS (mips): Remove myself as MIPS maintainer.



RE: [PATCH mips] Do not compile mips16.S in soft-float mode

2014-08-09 Thread Moore, Catherine


> -Original Message-
> From: Steve Ellcey [mailto:sell...@mips.com]
> Subject: [PATCH mips] Do not compile mips16.S in soft-float mode
> 
> This libgcc patch is the second of two patches needed to build GCC soft-float
> multilibs when using the latest binutils.  It skips assembling any of the 
> code in
> mips16.S when in soft-float mode because the code is not used when in soft-
> float mode and because doing so generates errors during assembly (due to
> using floating point registers in soft-float mode).
> 
> Tested with the mips-mti-linux-gnu toolchain.
> 

FWIW, this patch looks okay to me (although I can't approve it).
Catherine

> 
> 2014-08-08  Steve Ellcey  
> 
>   * config/mips/mips16.S:  Skip when __mips_soft_float is defined.
> 
> 
> diff --git a/libgcc/config/mips/mips16.S b/libgcc/config/mips/mips16.S index
> 6a43a98..150a23a 100644
> --- a/libgcc/config/mips/mips16.S
> +++ b/libgcc/config/mips/mips16.S
> @@ -21,7 +21,7 @@ a copy of the GCC Runtime Library Exception along with
> this program;  see the files COPYING3 and COPYING.RUNTIME respectively.
> If not, see  .  */
> 
> -#ifdef __mips_micromips
> +#if defined(__mips_micromips) || defined(__mips_soft_float)
>/* DO NOTHING */
>  #else
> 
> @@ -749,4 +749,4 @@ CALL_STUB_RET (__mips16_call_stub_dc_10, 10, DC)
> #endif /* !__mips_single_float */
> 
>  #endif
> -#endif /* __mips_micromips */
> +#endif /* defined(__mips_micromips) || defined(__mips_soft_float) */


RE: [PATCH mips] Pass -msoft-float/-mhard-float flags to GAS

2014-08-09 Thread Moore, Catherine


> -Original Message-
> From: Steve Ellcey [mailto:sell...@mips.com]
> Sent: Friday, August 08, 2014 3:42 PM
> To: Moore, Catherine; matthew.fort...@imgtec.com; echri...@gmail.com;
> 
> 2014-08-08  Steve Ellcey  
> 
>   * config/mips/mips.h (ASM_SPEC): Pass float options to assembler.
> 
> diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> 8d7a09f..9a15287 100644
> --- a/gcc/config/mips/mips.h
> +++ b/gcc/config/mips/mips.h
> @@ -1187,6 +1187,8 @@ struct mips_cpu_info {  %{mshared} %{mno-
> shared} \  %{msym32} %{mno-sym32} \  %{mtune=*} \
> +%{mhard-float} %{msoft-float} \
> +%{msingle-float} %{mdouble-float} \
>  %(subtarget_asm_spec)"
> 
>  /* Extra switches sometimes passed to the linker.  */
> 

Hi Steve,
The patch itself looks okay, but perhaps a question for Matthew.
Does the fact that the assembler requires -msoft-float even if .set softfloat 
is present in the .s file deliberate behavior?
I don't have a problem with passing along the *float* options to gas, but would 
hope that the .set options were honored as well.
My short test indicated that the .set *float* options were being ignored if the 
correct command line option wasn't present.
Thanks,
Catherine



RE: [PATCH mips] Pass -msoft-float/-mhard-float flags to GAS

2014-08-10 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Saturday, August 09, 2014 3:00 PM
> To: Moore, Catherine; Steve Ellcey; echri...@gmail.com; GCC Patches
> Subject: RE: [PATCH mips] Pass -msoft-float/-mhard-float flags to GAS
> 
> Moore, Catherine  writes:
> > > -Original Message-
> > > From: Steve Ellcey [mailto:sell...@mips.com]
> > > Sent: Friday, August 08, 2014 3:42 PM
> > > To: Moore, Catherine; matthew.fort...@imgtec.com;
> > > echri...@gmail.com;
> > >
> > > 2014-08-08  Steve Ellcey  
> > >
> > >   * config/mips/mips.h (ASM_SPEC): Pass float options to assembler.
> > >
> > > diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> > > 8d7a09f..9a15287 100644
> > > --- a/gcc/config/mips/mips.h
> > > +++ b/gcc/config/mips/mips.h
> > > @@ -1187,6 +1187,8 @@ struct mips_cpu_info {  %{mshared} %{mno-
> > > shared} \  %{msym32} %{mno-sym32} \  %{mtune=*} \
> > > +%{mhard-float} %{msoft-float} \
> > > +%{msingle-float} %{mdouble-float} \
> > >  %(subtarget_asm_spec)"
> > >
> > >  /* Extra switches sometimes passed to the linker.  */
> > >
> >
> > Hi Steve,
> > The patch itself looks okay, but perhaps a question for Matthew.
> > Does the fact that the assembler requires -msoft-float even if .set
> > softfloat is present in the .s file deliberate behavior?
> 
> The assembler requires -msoft-float if .gnu_attribute 4,3 is given. I.e. the
> overall module options must match the ABI which has been specified. .set
> directives can still be used to override the 'current' options and be
> inconsistent with the overall module and/or .gnu_attribute setting.
> 
> > I don't have a problem with passing along the *float* options to gas,
> > but would hope that the .set options were honored as well.
> 
> Yes they should be.
> 
> > My short test indicated that the .set *float* options were being
> > ignored if the correct command line option wasn't present.
> 
> I'm not certain what you are describing here. Could you confirm with an
> example just in case something is not working as expected?

I don't have the example that I tried yesterday, but I am not able to reproduce 
today.
If I notice again, I will post the example.
Thanks,
Catherine


RE: [PATCH] Workaround errata for the PMC-Sierra RM7000 cpu.

2013-11-19 Thread Moore, Catherine
> -Original Message-
> From: Moore, Catherine
> Subject: RE: [PATCH] Workaround errata for the PMC-Sierra RM7000 cpu.
> 
> writes:
> > >> -Original Message-
> > >> From: Richard Sandiford [mailto:rdsandif...@googlemail.com]
> > >> Subject: Re: [PATCH] Workaround errata for the PMC-Sierra RM7000
> cpu.
> > >>
> > >> "Moore, Catherine"  writes:
> > >> > Hi Richard,
> > >> >
> > >> > This patch implements a workaround for errors on the PMC-Sierra
> > >> > RM7000 cpu while executing the dmult or dmultu instruction.  The
> > >> > workaround is to insert three nops after the dmult/dmultu.
> > >>
> > >> Do you have any more details?  E.g. does "dmult $4,$5;addiu $6,$7,1"
> > >> cause problems?  The VR41xx series had errata like these, but it
> > >> was always between the multiplication and other references to HI/LO.
> > >

This is an updated patch that recognizes the -mifx-rm7000 option and passes it 
off to the assembler.

Okay to install?
Thanks,
Catherine



pmc.cl
Description: pmc.cl


pmc.patch
Description: pmc.patch


RE: [RFC]: Remove Mem/address type assumption in combiner

2015-05-12 Thread Moore, Catherine


> -Original Message-
> From: Steve Ellcey [mailto:sell...@imgtec.com]
> Sent: Tuesday, May 12, 2015 12:51 PM
> To: Kumar, Venkataramanan
> Cc: Segher Boessenkool; Jeff Law (l...@redhat.com); gcc-
> patc...@gcc.gnu.org; maxim.kuvyr...@linaro.org; Matthew Fortune;
> Moore, Catherine
> Subject: RE: [RFC]: Remove Mem/address type assumption in combiner
> 
> On Tue, 2015-05-12 at 05:21 +, Kumar, Venkataramanan wrote:
> > Hi Steve,
> >
> > Yes this is expected.  As Segher pointed out, we need to change .md
> > patterns in target to be based on shifts instead of mults.
> >
> > Regards,
> > Venkat.
> 
> Here is my patch to change this.  I tested it with the mips-mti-linux-gnu and
> mips-mti-elf targets and it fixed the MIPS specific tests that were scanning
> for an lsa instruction and it also had no regressions.
> 
> Since the lsa instruction was the only one using the 'y' operand code and my
> change got rid of the only use of it, I removed it as part of this patch.
> 
> Matthew or Catherine is this OK to checkin?
> 
> 2015-05-12  Steve Ellcey  
> 
>   * config/mips/mips.c (mips_print_operand): Remove 'y' operand
> code.
>   * config/mips/mips.md (lsa): Rewrite with shift operator.
>   * config/mips/predicates.md (const_immlsa_operand): Remove log
> call.
> 

This is OK.
Catherine


RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

2015-04-16 Thread Moore, Catherine


> -Original Message-
> From: Maciej W. Rozycki [mailto:ma...@linux-mips.org]
> Sent: Thursday, April 16, 2015 2:23 PM
> To: Petar Jovanovic
> Cc: Moore, Catherine; 'Matthew Fortune'; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> 
> On Thu, 16 Apr 2015, Petar Jovanovic wrote:
> 
> > > There isn't any need to execute a large testcase.  Instead, try
> > > adding a
> > short version of your test to the directory gcc/testsuite/gcc.target/mips.
> > > There are examples of other tests there they check for specific
> > > assembler
> > sequences by using scan-assembler.   See umips-swp-1.c (and others) for a
> > specific example of how to do this.
> > > Let me know if you need additional help.
> > > Thanks,
> > > Catherine
> >
> > Hi Catherine,
> >
> > Sorry for late response, I have missed to follow up on this.
> > IIUC, scan-assembler will scan assembly file generated from a test file.
> > This particular change will - on the other hand - modify crtend.o and
> > thus init section. Is there a 'scan-assembler' functionality over a
> > final linked executable, as that would actually test the change?
> 
Hi Petar,
It looks like I answered a little too quickly the first time around.  I'm sorry.
In any case, Maciej is correct.  I think a link-time test should do it.
Thanks,
Catherine

>  You'd need `objdump' for doing that and there is no ready-to-use solution
> within the GCC test suite available, you'd have to cook something up
> yourself, perhaps starting with `[find_binutils_prog objdump]'.
> 
>  Another solution might be using an auxiliary linker script (`-Wl,-T,...'
> or maybe just an implicit linker script will do; see the LD manual for
> details) to place the sections the jump is made between apart manually at
> addresses appropriate for JAL to fail.  They span does not have to be large --
> when placed correctly, even `JAL .' can jump to the wrong place, which is
> what LD is supposed to catch as a relocation error -- so a test executable
> successfully linked with your correction in place won't be large, it doesn't
> have to take more than 2 virtual pages.
> 
>  The downside of the latter solution are possible portability issues with the
> linker script.  You won't have to run your executable AFAICT from glibc BZ
> #17601 as you'll get a link error if a jump target is out of range.  So you 
> could
> make it a mere link test, no need to run it.
> 
>   Maciej


RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

2015-04-17 Thread Moore, Catherine


> -Original Message-
> From: Petar Jovanovic [mailto:petar.jovano...@rt-rk.com]
> Sent: Friday, April 17, 2015 2:23 PM
> To: Petar Jovanovic
> Cc: Maciej W. Rozycki; Matthew Fortune; gcc-patches@gcc.gnu.org; Moore,
> Catherine
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> 
> Resending the previous message in a plain text format.
> 
> >  Original Message 
> > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> > Date: Thursday, April 16, 2015 10:38 PM CEST
> > From: "Moore, Catherine" 
> > To: "Maciej W. Rozycki" , Petar
> > Jovanovic
> > CC: 'Matthew Fortune' ,
> > "gcc-patches@gcc.gnu.org"
> > References:
> > <003e01d04179$ccc38bc0$664aa340$@rt-
> rk.com><6D39441BF12EF246A7ABCE6654
> >
> b0235320fca...@lemail01.le.imgtec.org><000201d04723$f69b1350$e3d13
> 9f0$
> > @rt-rk.com><000501d07865$e26f2830$a74d7890$@rt-rk.com>
> > Hi Petar,
> > It looks like I answered a little too quickly the first time around. I'm 
> > sorry.
> > In any case, Maciej is correct. I think a link-time test should do it.
> > Thanks,
> > Catherine
> >
> > > You'd need `objdump' for doing that and there is no ready-to-use
> > > solution within the GCC test suite available, you'd have to cook
> > > something up yourself, perhaps starting with `[find_binutils_prog
> objdump]'.
> > >
> > > Another solution might be using an auxiliary linker script (`-Wl,-T,...'
> > > or maybe just an implicit linker script will do; see the LD manual
> > > for
> > > details) to place the sections the jump is made between apart
> > > manually at addresses appropriate for JAL to fail. They span does
> > > not have to be large -- when placed correctly, even `JAL .' can jump
> > > to the wrong place, which is what LD is supposed to catch as a
> > > relocation error -- so a test executable successfully linked with
> > > your correction in place won't be large, it doesn't have to take more than
> 2 virtual pages.
> > >
> > > The downside of the latter solution are possible portability issues
> > > with the linker script. You won't have to run your executable AFAICT
> > > from glibc BZ
> > > #17601 as you'll get a link error if a jump target is out of range.
> > > So you could make it a mere link test, no need to run it.
> > >
> > > Maciej
> >
> >
> 
>  Hi Maciej, Catherine,
> 
>  This issue will not trigger a linker error (I believe it treats the  symbol
> referred by the relocation as a local symbol). This is a follow  up to GLIBC 
> BZ
> #17601, the problem is seen only at runtime. So, I think  this brings back the
> need to run the test. I can look into separating  sections with -Wl,-T.. (that
> may also require extending  mips_option_groups in mips/mips.exp), if
> running executable is OK.
> 
Hi Petar,  
Running the executable is fine, but didn't you say that your test case takes 
hours to execute?
If so, that's not acceptable.  Is it possible to construct a test case that 
will run more quickly?
Thanks,
Catherine


RE: [PATCH][MIPS] Enable load-load/store-store bonding

2015-04-20 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Monday, April 20, 2015 3:10 PM
> To: Sameera Deshpande; Moore, Catherine
> Cc: Richard Sandiford; gcc-patches@gcc.gnu.org; echri...@gmail.com
> Subject: RE: [PATCH][MIPS] Enable load-load/store-store bonding
> 
> Sameera Deshpande  writes:
> > Gentle reminder!
> 
> Thanks Sameera. Just a couple of comments inline below and a question for
> Catherine at the end.
> 
> > - Thanks and regards,
> >Sameera D.
> >
> > On Monday 30 March 2015 04:58 PM, sameera wrote:
> > > Hi!
> > >
> > > Sorry for delay in sending this patch for review.
> > > Please find attached updated patch.
> > >
> > > In P5600, 2 consecutive loads/stores of same type which access
> > > contiguous memory locations are bonded together by instruction issue
> > > unit to dispatch single load/store instruction which accesses both
> > locations. This allows 2X improvement in memory intensive code. This
> > optimization can be performed for LH, SH, LW, SW, LWC, SWC, LDC, SDC
> > instructions.
> > >
> > > This patch adds peephole2 patterns to identify such loads/stores,
> > > and put them in parallel, so that the scheduler will not split it -
> > thereby guaranteeing h/w level load/store bonding.
> > >
> > > The patch is tested with dejagnu for correctness, and tested on
> > hardware for performance.
> > > Ok for trunk?
> > >
> > > Changelog:
> > > gcc/
> > >  * config/mips/mips.md (JOIN_MODE): New mode iterator.
> > >  (join2_load_Store): New pattern.
> > >  (join2_loadhi): Likewise.
> > >  (define_peehole2): Add peephole2 patterns to join 2
> > > HI/SI/SF/DF-
> > mode
> > >  load-load and store-stores.
> > >  * config/mips/mips.opt (mload-store-pairs): New option.
> > >  (TARGET_LOAD_STORE_PAIRS): New macro.
> > >  *config/mips/mips.h (ENABLE_LD_ST_PAIRS): Likewise.
> > >  *config/mips/mips-protos.h (mips_load_store_bonding_p): New
> > prototype.
> > >  *config/mips/mips.c(mips_load_store_bonding_p): New function.
> 
> I don't know if this has been corrupted by mail clients but a single space 
> after
> '*' and a space before '('.
> 
> >diff --git a/gcc/config/mips/mips-protos.h
> >b/gcc/config/mips/mips-protos.h index b48e04f..244eb8d 100644
> >--- a/gcc/config/mips/mips-protos.h
> >+++ b/gcc/config/mips/mips-protos.h
> >@@ -360,6 +360,7 @@ extern bool mips_epilogue_uses (unsigned int);
> >extern void mips_final_prescan_insn (rtx_insn *, rtx *, int);  extern
> >int mips_trampoline_code_size (void);  extern void
> >mips_function_profiler (FILE *);
> >+extern bool mips_load_store_bonding_p (rtx *, machine_mode, bool);
> >
> > typedef rtx (*mulsidi3_gen_fn) (rtx, rtx, rtx);  #ifdef RTX_CODE diff
> >--git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> >1733457..85f0591 100644
> >--- a/gcc/config/mips/mips.c
> >+++ b/gcc/config/mips/mips.c
> >@@ -18241,6 +18241,64 @@ umips_load_store_pair_p_1 (bool load_p,
> bool swap_p,
> >   return true;
> > }
> >
> >+bool
> >+mips_load_store_bonding_p (rtx *operands, enum machine_mode
> mode, bool
> >+load_p)
> 
> Remove enum from machine_mode.
> 
> >+{
> >+  rtx reg1, reg2, mem1, mem2, base1, base2;
> >+  enum reg_class rc1, rc2;
> >+  HOST_WIDE_INT offset1, offset2;
> >+
> >+  if (load_p)
> >+{
> >+  reg1 = operands[0];
> >+  reg2 = operands[2];
> >+  mem1 = operands[1];
> >+  mem2 = operands[3];
> >+}
> >+  else
> >+{
> >+  reg1 = operands[1];
> >+  reg2 = operands[3];
> >+  mem1 = operands[0];
> >+  mem2 = operands[2];
> >+}
> >+
> >+  if (mips_address_insns (XEXP (mem1, 0), mode, false) == 0
> >+  || mips_address_insns (XEXP (mem2, 0), mode, false) == 0)
> >+return false;
> >+
> >+  mips_split_plus (XEXP (mem1, 0), &base1, &offset1);  mips_split_plus
> >+ (XEXP (mem2, 0), &base2, &offset2);
> >+
> >+  /* Base regs do not match.  */
> >+  if (!REG_P (base1) || !rtx_equal_p (base1, base2))
> >+return false;
> >+
> >+  /* Either of the loads is clobbering base register.  */
> >+  if (load_p
> >+  && (REGNO (reg1) == REGNO (base1)
> >+  || (REGNO (reg2) == REGNO (base1
> >+return false;
> 
> Can you a

RE: [PATCH v4][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

2015-04-29 Thread Moore, Catherine


> -Original Message-
> From: Petar Jovanovic [mailto:petar.jovano...@rt-rk.com]
> Sent: Friday, April 24, 2015 10:51 AM
> Subject: [PATCH v4][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> 
> gcc/ChangeLog:
> 
> 2015-04-21  Petar Jovanovic  
> 
>   * config/mips/mips.h (CRT_CALL_STATIC_FUNCTION): Fix the macro
> to use
>   la/jalr instead of jal.
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-04-21  Petar Jovanovic  
> 
>   * gcc.target/mips/call-from-init.c: New test.
>   * gcc.target/mips/mips.exp: Add section_start to


Thank you, Petar.  I have now committed this patch.
Catherine



RE: [Patch, MIPS] Change mips4 default processor to r10K

2015-04-29 Thread Moore, Catherine
> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Tuesday, April 28, 2015 2:38 PM
> 
> Steve Ellcey   writes:
> >
> > 2015-04-28  Steve Ellcey  
> >
> > * config/mips/mips-cpus.def: (mips4): Change default processor
> > from PROCESSOR_R8000 to PROCESSOR_R1.
> 
> Ok with me. I'd like Catherine to have the chance to raise any concerns
> though.

This change is OK with me, as well.
Catherine



RE: [patch, MIPS, testsuite] Fix gcc.target/mips/branch-1.c

2015-05-11 Thread Moore, Catherine


> -Original Message-
> From: Steve Ellcey [mailto:sell...@imgtec.com]
> Sent: Monday, May 11, 2015 12:02 PM
> To: gcc-patches@gcc.gnu.org
> Cc: matthew.fort...@imgtec.com; Moore, Catherine
> Subject: [patch, MIPS, testsuite] Fix gcc.target/mips/branch-1.c
> 
> The test gcc.target/mips/branch-1.c has started failing because it is trying 
> to
> verify that each of 4 functions generates and 'andi' instruction and only
> finding 2 of them.  With a recent change (fixing PR 65150) GCC determined
> that the f1 and f2 functions generate identical code and that the f3 and f4
> functions generate identical code and so it replaced f1 with a tail call to 
> f2 and
> f3 with a tail call to f4.  Thus only 2 'andi' instructions show up and the 
> test
> fails.
> 
> There is an existing bug report (PR 65534) about whether or not this is a good
> optimization but I would like to fix this test so that that optimization can 
> not
> happen since that is not what this test is intended to check for.  My solution
> is to pass different arguments to bar() in each function so the code in each
> function is unique.
> 
> Tested with mips-mti-linux-gnu, OK to checkin?
> 
> Steve Ellcey
> sell...@imgtec.com
> 
> 
> 2015-05-11  Steve Ellcey  
> 
>   * gcc.target/mips/branch-1.c: Pass argument to bar().
> 
> 
This is OK.
Catherine


RE: [PATCH] microMIPS/GCC: Correct 64-bit instruction sizes

2014-11-17 Thread Moore, Catherine
Hi Maciej,

> -Original Message-
> From: Rozycki, Maciej
> Sent: Monday, November 17, 2014 2:39 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Moore, Catherine; Eric Christopher; Matthew Fortune
> Subject: [PATCH] microMIPS/GCC: Correct 64-bit instruction sizes
> 
> Hi,
> 
>  Noticed in an attempt to build a 64-bit microMIPS Linux kernel.
> 
>  None of the 64-bit operations have a short instruction encoding in the
> microMIPS instruction set.  Despite that our code has been written such that
> the presence of a short encoding is assumed for both 32-bit and 64-bit
> operations.  This leads to assembly warnings like:
> 
> {standard input}: Assembler messages:
> {standard input}:146: Warning: wrong size instruction in a 16-bit branch delay
> slot {standard input}:544: Warning: wrong size instruction in a 16-bit branch
> delay slot {standard input}:1071: Warning: wrong size instruction in a 16-bit
> branch delay slot {standard input}:1088: Warning: wrong size instruction in a
> 16-bit branch delay slot {standard input}:1833: Warning: wrong size
> instruction in a 16-bit branch delay slot
> 
> This is because code in question produced by GCC looks like:
> 
>   jalsget_option   #
>   daddiu  $4,$sp,16#,,
> 
> (in a `.set noreorder' fragment) and there is no short encoding for the
> DADDIU instruction available.
> 
>  At least two approaches are possible to address this problem, either by
> splitting the iterated patterns affected into separate 32-bit and 64-bit
> patterns, or by limiting the affected alternatives to 32-bit operations only. 
>  I
> found the latter less intrusive, with the use of an extra `compression'
> attribute value: `micromips32'.  

I prefer the second approach as well.

This is implemented with the change below,
> fixing the issues seen in 64-bit compilation.  Code produced now looks like:
> 
>   jal get_option   #
>   daddiu  $4,$sp,16#,,
> 
> instead.
> 
>  Some operations are operand size agnostic, these include LI16, MOVE16, and
> all the relevant logical instructions.  These retain the `micromips'
> setting for the `compression' attribute.  MOVEP is also operand size agnostic,
> but it must not be scheduled into a delay slot and it is therefore handled
> separately, with no `compression' attribute defined.
> 
>  Regression-tested with the mips-linux-gnu target and these multilibs:
> 
> -EB
> -EB -mips16
> -EB -mmicromips
> -EB -mabi=n32
> -EB -mabi=64
> 
> and the -EL variants of same, with no changes in results.  I have also checked
> that microMIPS shared glibc libraries have not changed when rebuilt with the
> updated compiler.
> 
>  OK to apply?
> 
> 2014-11-14  Maciej W. Rozycki  
> 
>   gcc/
>   * config/mips/mips.md (compression): Add `micromips32' setting.
>   (enabled, length): Handle it.
>   (shift_compression): Replace `micromips' with `micromips32' in
>   the `compression' attribute.
>   (*add3, sub3): Likewise.

Yes, this looks good.
Thanks,
Catherine



RE: [PATCH] MIPS/GCC: Unconditional jump generation bug fix

2014-11-18 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Tuesday, November 18, 2014 12:22 PM
> To: Rozycki, Maciej
> Cc: gcc-patches@gcc.gnu.org; Moore, Catherine; Eric Christopher
> Subject: RE: [PATCH] MIPS/GCC: Unconditional jump generation bug fix
> 
> Maciej W. Rozycki  writes:
> > On Mon, 17 Nov 2014, Matthew Fortune wrote:
> >
> > > > gcc/
> > > > * gcc/config/mips/mips.md (*jump_absolute): Use a branch when in
> > > > range, a jump otherwise.
> > >
> > > OK.
> > >
> > > I only got my head around this code last week otherwise I wouldn't
> > > have known whether this was correct!
> >
> >  Committed now, thanks for your review.  How about 4.9? -- once it is
> > unfrozen, that is.
> 
> I admit to being a bit more nervous about 4.9 but the test coverage seems
> thorough enough. I guess I would have been less concerned if the
> optimisation was still just tied to TARGET_MICROMIPS for the 4.9 branch.
> 
> Catherine, what do you think?
> 
This is okay for 4.9 IMO.


RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-19 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Tuesday, November 18, 2014 9:42 AM
> To: Andrew Bennett; gcc-patches@gcc.gnu.org
> Cc: Moore, Catherine; Rozycki, Maciej
> Subject: RE: [PATCH] If using branch likelies in MIPS sync code fill the delay
> slot with a nop
> 
> > The atomic-compare-exchange-3.c and atomic-op-3.c tests are failing
> > when using the -mfix-r1 option.  This is due to the fact that the
> > delay slot of the branch instruction that checks if the atomic
> > operation was not successful can be filled with an operation that
> > returns the output result.  For the branch likely case this operation
> > can not go in the delay slot because it wont execute when the atomic
> > operation was successful and therefore the wrong result will be
> > returned.  This patch forces a nop to be placed in the delay slot if a 
> > branch
> likely is used.
> > The fix is as simple as possible; it does cause a second nop to be
> > introduced after the branch instruction in the branch likely case.  As
> > this option is rarely used, it makes more sense to keep the code
> > readable than trying to optimise it.
> >
> > The atomic tests now pass successfully.  The ChangeLog and patch are
> > below.
> >
> > Ok to commit?
> 
> I'm OK with just making the fix-r1 case safe rather than also complicating
> the normal code path to remove the normal delay slot NOP in this special
> case.  I'd like to see what Catherine thinks though.  To remove the second
> NOP I believe we would have to add a !TARGET_FIX_R1 to the condition
> of the normal delay slot NOP. This seems a bit convoluted when the real
> reason is because branch likely is in use.  To make use of the
> mips_branch_likely flag it would have to be set earlier in:
> mips_output_sync_loop and also get set in mips_sync_loop_insns.
> 
> If Catherine thinks getting rid of the second NOP is important enough then
> please base it on mips_branch_likely and fix the callers.
> 
  Yes, removing the second NOP is worth the additional effort.


RE: [PATCH] MIPS16/GCC: Optimise `__call_stub_fp_' call/return stubs

2014-11-21 Thread Moore, Catherine


> -Original Message-
> From: Rozycki, Maciej
> Sent: Wednesday, November 19, 2014 8:05 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Moore, Catherine; Eric Christopher; Matthew Fortune
> Subject: [PATCH] MIPS16/GCC: Optimise `__call_stub_fp_' call/return stubs
> 
> 
> 2014-11-19  Maciej W. Rozycki  
> 
>   gcc/
>   * config/mips/mips.c (mips16_build_call_stub): Move the save of
>   the return address in $18 ahead of passing arguments to FPRs.
> 
>   Maciej

This looks OK.  Please commit.
 


RE: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-madd

2015-06-29 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Monday, June 29, 2015 3:07 PM
> To: Maciej W. Rozycki; Steve Ellcey; Moore, Catherine; Richard Biener
> Cc: Richard Sandiford; Richard Sandiford; Myers, Joseph; gcc-
> patc...@gcc.gnu.org
> Subject: RE: [Patch, MIPS] Enable fp-contract on MIPS and update -mfused-
> madd
> 
> Maciej W. Rozycki  writes:
> > Richard, please have a look at my question below in a reference to
> > your previous statement.
> >
> > On Thu, 18 Jun 2015, Steve Ellcey wrote:
> >
> > > OK, I checked in the prequel patch and here is a new copy of the
> > > original patch based off of that (and with no HONOR_NAN checks in
> > > the fma/madd instructions).
> > >
> > > OK for checkin?
> >
> >  Please see below for my notes.
> >
> > > 2015-06-18  Steve Ellcey  
> > >
> > >   * config.gcc (mips*-*-*): Add fused-madd.opt.
> >
> >  Please use angle brackets as per
> > <https://www.gnu.org/prep/standards/html_node/Indicating-the-Part-
> Chan
> > ged.html>,
> > i.e.:
> >
> > * config.gcc : Add fused-madd.opt.
> >
> > There's no function or similar entity involved here and `mips*-*-*' is
> > a case value like with the C language's `switch' statement where you'd
> > use angle brackets too to refer to individual cases.
> >
> > >   (*nmsub4_fastmath)  Update condition.
> >
> >  Extraneous space here.
> >
> >  The change looks good to me otherwise.
> >
> >   Maciej
> 
> If nobody can identify any further functional issues with this patch then I'd
> like to get this committed and pursue enhancements as a second round.
> Catherine, would you be happy for this to be committed on that basis?
> 
Yes, this is fine with me.


RE: [PATCH] MIPS: For micromips allow near-far-3.c test to use the jals instruction to call near_func

2015-07-06 Thread Moore, Catherine


> -Original Message-
> From: Andrew Bennett [mailto:andrew.benn...@imgtec.com]
> Sent: Monday, July 06, 2015 9:20 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Matthew Fortune; Moore, Catherine
> Subject: [PATCH] MIPS: For micromips allow near-far-3.c test to use the jals
> instruction to call near_func
> 
> Hi,
> 
> The near-far-3.c test is failing for micromips because it is expecting the 
> call to
> near_func to be performed by a jal instruction, but for micromips this is done
> by a jals instruction.
> 
> I have updated the expected test output to deal with this case.  I have tested
> this on the mips-mti-elf target using mips32r2/{-mno-micromips/-
> mmicromips}
> test options and there are no new regressions.
> 
> The patch and ChangeLog are below.
> 
> Ok to commit?
> 
> testsuite/
>   * gcc.target/mips/near-far-3.c: Allow the call to near_func to use
> the jals instruction.
> 
> 

OK.


RE: [PATCH] MIPS: Update stack-1.c testcase to match micromips jraddiusp instruction.

2015-07-06 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Monday, July 06, 2015 11:00 AM
> To: Andrew Bennett; gcc-patches@gcc.gnu.org
> Cc: Moore, Catherine
> Subject: RE: [PATCH] MIPS: Update stack-1.c testcase to match micromips
> jraddiusp instruction.
> 
> Andrew Bennett  writes:
> > The stack-1.c testcase fails when being compiled for micromips with
> > the
> > -O0 optimization level.  The reason is the testcase is expecting the
> > following sequence at the end of the function:
> >
> >addiu   $sp,$sp,16
> >jrc $31
> >
> > But for micromips it generates the following:
> >
> >jraddiusp   16
> >
> >
> > As the failure only happens at one optimization level I have decided
> > to just change the expected output rather than creating a separate
> > micromips testcase.
> 
> I'm not sure this is the right approach here. If we get a jraddiusp then the
> problem that the test is trying to cover can't possibly happen anyway.
> (The test is checking if a load and final stack adjustment are ever re-ordered
> from what I can see.)
> 
> I'd just mark the test as NOCOMPRESSION instead of just NOMIPS16 and
> update the comment to say that it is avoiding SAVE, RESTORE and
> JRADDIUSP.
> 

Another approach would be to add the micromips testcase variant and skip the 
test if code-quality (ie. -O0).
Catherine


RE: [PATCH] MIPS: Do not generate micromips code for the no-smartmips-lwxs.c testcase

2015-07-06 Thread Moore, Catherine


> -Original Message-
> From: Andrew Bennett [mailto:andrew.benn...@imgtec.com]
> Sent: Monday, July 06, 2015 9:34 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Matthew Fortune; Moore, Catherine
> Subject: [PATCH] MIPS: Do not generate micromips code for the no-
> smartmips-lwxs.c testcase
> 
> Hi,
> 
> The LWXS instruction is part of the micromips ISA which means it is valid to
> generate it for the no-smartmips-lwxs.c testcase.  I have updated the dg-
> options for the test to ensure that it does not generate micromips code.
> 
> I have tested this on the mips-mti-elf target using mips32r2/{-mno-
> micromips/-mmicromips} test options and there are no new regressions.
> 
> The patch and ChangeLog are below.
> 
> Ok to commit?
> 
> 
> 
> Many thanks,
> 
> 
> Andrew
> 
> 
> 
> testsuite/
>   * gcc.target/mips/no-smartmips-lwxs.c: Add -mno-micromips to dg-
> options.
> 
> 
> diff --git a/gcc/testsuite/gcc.target/mips/no-smartmips-lwxs.c
> b/gcc/testsuite/gcc.target/mips/no-smartmips-lwxs.c
> index ecf856e..6701a1c 100644
> --- a/gcc/testsuite/gcc.target/mips/no-smartmips-lwxs.c
> +++ b/gcc/testsuite/gcc.target/mips/no-smartmips-lwxs.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-mno-smartmips" } */
> +/* { dg-options "-mno-smartmips -mno-micromips" } */
> 
>  NOMIPS16 int scaled_indexed_word_load (int a[], int b)  {
> 

Hi Andrew,

Instead of adding the -mno-micromips option to dg-options, please change the 
MIPS16 attribute to NOCOMPRESSION.

Index: gcc.target/mips/no-smartmips-lwxs.c
===
--- gcc.target/mips/no-smartmips-lwxs.c (revision 452061)
+++ gcc.target/mips/no-smartmips-lwxs.c (working copy)
@@ -1,7 +1,7 @@
 /* { dg-do compile } */
 /* { dg-options "-mno-smartmips" } */

-NOMIPS16 int scaled_indexed_word_load (int a[], int b)
+NOCOMPRESSION int scaled_indexed_word_load (int a[], int b)
 {
   return a[b];
 }

OK with that change.
Catherine



RE: [PATCH] MIPS: fix failing branch range checks for micromips

2015-07-06 Thread Moore, Catherine
Hi Andrew,

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Andrew Bennett
> Sent: Friday, July 03, 2015 7:50 PM
> Subject: [PATCH] MIPS: fix failing branch range checks for micromips
> 
> diff --git a/gcc/testsuite/gcc.target/mips/branch-10.c
> b/gcc/testsuite/gcc.target/mips/branch-10.c
> index e2b1b5f..00569b0 100644
> --- a/gcc/testsuite/gcc.target/mips/branch-10.c
> +++ b/gcc/testsuite/gcc.target/mips/branch-10.c
> @@ -1,4 +1,4 @@
> -/* { dg-options "-mshared -mabi=n32" } */
> +/* { dg-options "-mshared -mabi=n32 -mno-micromips" } */
>  /* { dg-final { scan-assembler-not "(\\\$28|%gp_rel|%got)" } } */
>  /* { dg-final { scan-assembler-not "\tjr\t\\\$1\n" } } */
> 

Like the other patch, the -mno-micromips should be removed from dg-options and 
NOCOMPRESS used in place of NOMIPS16.
This comment applies to all of the branch-*.c tests.

> 
> diff --git a/gcc/testsuite/gcc.target/mips/branch-helper.h
> b/gcc/testsuite/gcc.target/mips/branch-helper.h
> index 85399be..bc4a31f 100644
> --- a/gcc/testsuite/gcc.target/mips/branch-helper.h
> +++ b/gcc/testsuite/gcc.target/mips/branch-helper.h
> @@ -33,5 +33,23 @@
> D2 ("nop") "\n\t" \
> D1 ("nop"))
> 
> +/* Emit something that is 0xfffc bytes long, which is the largest
> +   permissible range for micromips forward branches when branches

s/micromips/microMIPS/

> +   have delay slots.  */
> +#define OCCUPY_0xfffc \
> +  asm (D13 ("nop32") "\n\t" \
> +   D12 ("nop32") "\n\t" \
> +   D11 ("nop32") "\n\t" \
> +   D10 ("nop32") "\n\t" \
> +   D9 ("nop32") "\n\t" \
> +   D8 ("nop32") "\n\t" \
> +   D7 ("nop32") "\n\t" \
> +   D6 ("nop32") "\n\t" \
> +   D5 ("nop32") "\n\t" \
> +   D4 ("nop32") "\n\t" \
> +   D3 ("nop32") "\n\t" \
> +   D2 ("nop32") "\n\t" \
> +   D1 ("nop32") "\n\t" \
> +   D0 ("nop32"))
>  /* Likewise emit something that is 0x1fffc bytes long.  */  #define
> OCCUPY_0x1fffc do { asm ("nop"); OCCUPY_0x1fff8; } while (0)

diff --git
> a/gcc/testsuite/gcc.target/mips/branch-umips-10.c
> b/gcc/testsuite/gcc.target/mips/branch-umips-10.c

I see that you are naming these tests after the original branch- tests 
that they were derived from.
I think it would be better to keep all of the microMIPS tests named umips-???.  
  I don't think preserving the original number is important.

Thanks,
Catherine




RE: [PATCH] MIPS: Fix the call-[1,5,6].c tests to allow the jrc instruction to be matched when testing with microMIPS

2015-07-07 Thread Moore, Catherine


> -Original Message-
> From: Andrew Bennett [mailto:andrew.benn...@imgtec.com]
> Sent: Tuesday, July 07, 2015 6:53 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Moore, Catherine; Matthew Fortune
> Subject: [PATCH] MIPS: Fix the call-[1,5,6].c tests to allow the jrc 
> instruction
> to be matched when testing with microMIPS
> 
> Hi,
> 
> When building the call-[1,5,6].c tests for micromips the jrc rather than the 
> jr
> instruction is used to call the tail* functions.
> 
> I have updated the test output to allow the jrc instruction to be matched.
> 
> I have tested this on the mips-mti-elf target using mips32r2/{-mno-
> micromips/-mmicromips}
> test options and there are no new regressions.
> 
> The patch and ChangeLog are below.
> 
> Ok to commit?
> 
> 
> testsuite/
>   * gcc.target/mips/call-1.c: Allow testcase to match the jrc instruction.
>   * gcc.target/mips/call-5.c: Ditto.
>   * gcc.target/mips/call-6.c: Ditto.
> 
> 
OK.


RE: [PATCH] MIPS: Update stack-1.c testcase to match micromips jraddiusp instruction.

2015-07-07 Thread Moore, Catherine


> -Original Message-
> From: Andrew Bennett [mailto:andrew.benn...@imgtec.com]
> Sent: Tuesday, July 07, 2015 12:14 PM
> To: Moore, Catherine; Matthew Fortune; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] MIPS: Update stack-1.c testcase to match micromips
> jraddiusp instruction.
> 
> > > I'm not sure this is the right approach here. If we get a jraddiusp
> > > then the problem that the test is trying to cover can't possibly happen
> anyway.
> > > (The test is checking if a load and final stack adjustment are ever
> > > re-
> > ordered
> > > from what I can see.)
> > >
> > > I'd just mark the test as NOCOMPRESSION instead of just NOMIPS16 and
> > > update the comment to say that it is avoiding SAVE, RESTORE and
> > > JRADDIUSP.
> > >
> >
> > Another approach would be to add the micromips testcase variant and
> > skip the test if code-quality (ie. -O0).
> > Catherine
> 
> I agree with Matthew here.  The testcase already comments that it is
> preventing the use of the MIPS16 save and restore instructions, so it makes
> sense to prevent jraddiusp as well.
> 
> The updated patch and ChangeLog is below.
> 
> Ok to commit?
> 
> 
> 
> testsuite/
>   * gcc.target/mips/stack-1.c: Do not build the testcase for micromips.
> 
> 
Yes, this is OK.


RE: [PATCH] MIPS: fix failing branch range checks for micromips

2015-07-07 Thread Moore, Catherine
Hi Andrew,

> -Original Message-
> From: Andrew Bennett [mailto:andrew.benn...@imgtec.com]
> Sent: Tuesday, July 07, 2015 12:13 PM
> To: Moore, Catherine; gcc-patches@gcc.gnu.org
> Cc: Matthew Fortune
> Subject: RE: [PATCH] MIPS: fix failing branch range checks for micromips
> 
> 
> Ok to commit?
> 
> testsuite/
>   * gcc.target/mips/branch-2.c: Change NOMIPS16 to
> NOCOMPRESSION.
>   * gcc.target/mips/branch-3.c: Ditto
>   * gcc.target/mips/branch-4.c: Ditto.
>   * gcc.target/mips/branch-5.c: Ditto.
>   * gcc.target/mips/branch-6.c: Ditto.
>   * gcc.target/mips/branch-7.c: Ditto.
>   * gcc.target/mips/branch-8.c: Ditto.
>   * gcc.target/mips/branch-9.c: Ditto.
>   * gcc.target/mips/branch-10.c: Ditto.
>   * gcc.target/mips/branch-11.c: Ditto.
>   * gcc.target/mips/branch-12.c: Ditto.
>   * gcc.target/mips/branch-13.c: Ditto.

These are OK, except for the splitting of the scan-assembler statements.

Please change occurrences of:
> +/* { dg-final { scan-assembler
> +"\tld\t\\\$1,%got_page\\(\[^)\]*\\)\\(\\\$3\\)\\n" } } */
to: 
+/* { dg-final { scan-assembler 
"\tld\t\\\$1,%got_page\\(\[^)\]*\\)\\(\\\$3\\)\\n" } } */

before committing.


>   * gcc.target/mips/branch-14.c: Ditto.
>   * gcc.target/mips/branch-15.c: Ditto.

The modifications for these two files need to be removed.   These are execution 
tests and the multilib that is used to link them is important.   If the 
libraries are not compatible with the NOCOMPRESSION attribute, then the link 
step will fail.  You could work around this problem by enabling interlinking, 
but I think the best approach is to leave these two tests alone.

>   * gcc.target/mips/umips-branch-5.c: New file.
>   * gcc.target/mips/umips-branch-6.c: New file.
>   * gcc.target/mips/umips-branch-7.c: New file.
>   * gcc.target/mips/umips-branch-8.c: New file.
>   * gcc.target/mips/umips-branch-9.c: New file.
>   * gcc.target/mips/umips-branch-10.c: New file.
>   * gcc.target/mips/umips-branch-11.c: New file.
>   * gcc.target/mips/umips-branch-12.c: New file.
>   * gcc.target/mips/umips-branch-13.c: New file.
>   * gcc.target/mips/umips-branch-14.c: New file.
>   * gcc.target/mips/umips-branch-15.c: New file.
>   * gcc.target/mips/umips-branch-16.c: New file.

Same comment as above on the scan-assembler statements.

>   * gcc.target/mips/umips-branch-17.c: New file.
>   * gcc.target/mips/umips-branch-18.c: New file.

These two tests suffer from the same problem as above.  They should be deleted 
altogether.

>   * gcc.target/mips/branch-helper.h (OCCUPY_0x1): New define.
>   (OCCUPY_0xfffc): New define.

This is okay.

Thanks,
Catherine

> 
> diff --git a/gcc/testsuite/gcc.target/mips/branch-10.c
> b/gcc/testsuite/gcc.target/mips/branch-10.c
> index e2b1b5f..eb21c16 100644
> --- a/gcc/testsuite/gcc.target/mips/branch-10.c
> +++ b/gcc/testsuite/gcc.target/mips/branch-10.c
> @@ -4,7 +4,7 @@
> 
>  #include "branch-helper.h"
> 
> -NOMIPS16 void
> +NOCOMPRESSION void
>  foo (int (*bar) (void), int *x)
>  {
>*x = bar ();
> diff --git a/gcc/testsuite/gcc.target/mips/branch-11.c
> b/gcc/testsuite/gcc.target/mips/branch-11.c
> index 962eb1b..bd8e834 100644
> --- a/gcc/testsuite/gcc.target/mips/branch-11.c
> +++ b/gcc/testsuite/gcc.target/mips/branch-11.c
> @@ -8,7 +8,7 @@
> 
>  #include "branch-helper.h"
> 
> -NOMIPS16 void
> +NOCOMPRESSION void
>  foo (int (*bar) (void), int *x)
>  {
>*x = bar ();
> diff --git a/gcc/testsuite/gcc.target/mips/branch-12.c
> b/gcc/testsuite/gcc.target/mips/branch-12.c
> index 4aef160..4944634 100644
> --- a/gcc/testsuite/gcc.target/mips/branch-12.c
> +++ b/gcc/testsuite/gcc.target/mips/branch-12.c
> @@ -4,7 +4,7 @@
> 
>  #include "branch-helper.h"
> 
> -NOMIPS16 void
> +NOCOMPRESSION void
>  foo (int (*bar) (void), int *x)
>  {
>*x = bar ();
> diff --git a/gcc/testsuite/gcc.target/mips/branch-13.c
> b/gcc/testsuite/gcc.target/mips/branch-13.c
> index 8a6fb04..f5269b9 100644
> --- a/gcc/testsuite/gcc.target/mips/branch-13.c
> +++ b/gcc/testsuite/gcc.target/mips/branch-13.c
> @@ -8,7 +8,7 @@
> 
>  #include "branch-helper.h"
> 
> -NOMIPS16 void
> +NOCOMPRESSION void
>  foo (int (*bar) (void), int *x)
>  {
>*x = bar ();
> diff --git a/gcc/testsuite/gcc.target/mips/branch-14.c
> b/gcc/testsuite/gcc.target/mips/branch-14.c
> index 026417e..c2eecc3 100644
> --- a/gcc/testsuite/gcc.target/mips/branch-14.c
> +++ b/gcc/testsuite/gcc.target/mips/branch-14.c
> @@ -4,14 +4,14 @@
>  #include "branch-helper.h"
> 
>  void __attribut

RE: [PATCH] MIPS: fix failing branch range checks for micromips

2015-07-08 Thread Moore, Catherine


> -Original Message-
> From: Andrew Bennett [mailto:andrew.benn...@imgtec.com]
> Sent: Wednesday, July 08, 2015 11:17 AM
> To: Moore, Catherine; gcc-patches@gcc.gnu.org
> Cc: Matthew Fortune
> Subject: RE: [PATCH] MIPS: fix failing branch range checks for micromips
> 
> > > testsuite/
> > >   * gcc.target/mips/branch-2.c: Change NOMIPS16 to
> NOCOMPRESSION.
> > >   * gcc.target/mips/branch-3.c: Ditto
> > >   * gcc.target/mips/branch-4.c: Ditto.
> > >   * gcc.target/mips/branch-5.c: Ditto.
> > >   * gcc.target/mips/branch-6.c: Ditto.
> > >   * gcc.target/mips/branch-7.c: Ditto.
> > >   * gcc.target/mips/branch-8.c: Ditto.
> > >   * gcc.target/mips/branch-9.c: Ditto.
> > >   * gcc.target/mips/branch-10.c: Ditto.
> > >   * gcc.target/mips/branch-11.c: Ditto.
> > >   * gcc.target/mips/branch-12.c: Ditto.
> > >   * gcc.target/mips/branch-13.c: Ditto.
> >
> > These are OK, except for the splitting of the scan-assembler statements.
> >
> > Please change occurrences of:
> > > +/* { dg-final { scan-assembler
> > > +"\tld\t\\\$1,%got_page\\(\[^)\]*\\)\\(\\\$3\\)\\n" } } */
> > to:
> > +/* { dg-final { scan-assembler
> > "\tld\t\\\$1,%got_page\\(\[^)\]*\\)\\(\\\$3\\)\\n" } } */
> >
> > before committing.
> 
> I think this might be a problem with your email client, as these issues do not
> occur in my patch submission.
> 
Yes, it would appear that way.  Sorry for the noise.

> 
> 
> > >   * gcc.target/mips/branch-14.c: Ditto.
> > >   * gcc.target/mips/branch-15.c: Ditto.
> >
> > The modifications for these two files need to be removed.   These are
> > execution tests and the multilib that is used to link them is important.   
> > If
> > the libraries are not compatible with the NOCOMPRESSION attribute,
> > then the link step will fail.  You could work around this problem by
> > enabling interlinking, but I think the best approach is to leave these two
> tests alone.
> 
> Firstly, I have committed a patch which does not include the branch-[14,15].c
> and umips-branch-[17,18].c changes (SVN 225540).  However, I am keen to
> get these changes committed purely so that we have an in-range micromips
> branch execution test (which none of the current tests provide).  I need to
> look at the mips.exp file in more detail, but I was wondering if you would be
> happy to keep these tests in, but downgrade them to assemble tests if the
> required multilib support does not exist?
> 
How about adding the interlinking option to the umips-branch-[17,18].c tests 
instead?
Ie.  /* { dg-options "(-mmicromips) -minterlink-compressed" } */


 


RE: [MIPS] Update the ZC constraint for MIPSR6 and use it

2015-01-14 Thread Moore, Catherine
Hi Matthew,

> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Tuesday, January 06, 2015 7:43 AM
> To: Moore, Catherine
> Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> Subject: [MIPS] Update the ZC constraint for MIPSR6 and use it
> 
> Update the ZC constraint for MIPSR6 to allow it to be used as the memory
> operand for implementations of atomic operations.  Also switch the internal
> implementation of atomic operations to use ZC instead of ZR.
> 
> This fix accurately describes the memory constraints for the LL and SC
> instructions.  An offset can therefore be used to access a data item
> (ie. %lo ()) rather than always having to load the address into a
> register.  Tested for mips32r2, mips32r6 and micromips.
> 
> gcc/
> 
>   * config/mips/constraints.md (ZC): Add support for R6 LL/SC
>   offsets.
>   (ZD): Update to use ISA_HAS_PREF_LL_SC_9BIT.
>   * config/mips/mips.h (ISA_HAS_PREFETCH_9BIT): Rename to...
>   (ISA_HAS_PREF_LL_SC_9BIT): ... this. New macro.
>   * config/mips/sync.md (sync_compare_and_swap): Use ZC
>   instead of ZR for the memory operand of LL/SC.
>   (compare_and_swap_12, sync_add): Likewise.
>   (sync__12, sync_old__12): Likewise.
>   (sync_new__12, sync_nand_12): Likewise.
>   (sync_old_nand_12, sync_new_nand_12): Likewise.
>   (sync_sub, sync_old_add): Likewise.
>   (sync_old_sub, sync_new_add): Likewise.
>   (sync_new_sub, sync_): Likewise.
>   (sync_old_, sync_new_"):
> Likewise.
>   (sync_nand, sync_old_nand): Likewise.
>   (sync_new_nand, sync_lock_test_and_set):
> Likewise.
>   (test_and_set_12, atomic_compare_and_swap): Likewise.
>   (atomic_exchange_llsc, atomic_fetch_add_llsc):
> Likewise.
>   * doc/md.texi (ZC): Update description.
> 
> OK to commit?
> 
> diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
> index 9dad480..b608b17 100644
> --- a/gcc/config/mips/mips.h
> +++ b/gcc/config/mips/mips.h
> @@ -1089,8 +1089,8 @@ struct mips_cpu_info {
> || mips_isa_rev >= 1) \
>&& !TARGET_MIPS16)
> 
> -/* ISA has data prefetch with limited 9-bit displacement.  */
> -#define ISA_HAS_PREFETCH_9BIT(mips_isa_rev >= 6)
> +/* ISA has data prefetch, LL and SC with limited 9-bit displacement.  */
> +#define ISA_HAS_PREF_LL_SC_9BIT  (mips_isa_rev >= 6)
> 
I'd like to see this described as something more general.  Say:
ISA_HAS_9BIT_DISPLACEMENT.   This patch is okay with that fixup.
Thanks,
Catherine



RE: [MIPS] Update the ZC constraint for MIPSR6 and use it

2015-01-14 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Wednesday, January 14, 2015 2:54 PM
> To: Moore, Catherine
> Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> Subject: RE: [MIPS] Update the ZC constraint for MIPSR6 and use it
> 
> Moore, Catherine  writes
> > Hi Matthew,
> >
> > > -Original Message-
> > > From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> > > Sent: Tuesday, January 06, 2015 7:43 AM
> > > To: Moore, Catherine
> > > Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> > > Subject: [MIPS] Update the ZC constraint for MIPSR6 and use it
> > >
> > > Update the ZC constraint for MIPSR6 to allow it to be used as the
> > > memory operand for implementations of atomic operations.  Also
> > > switch the internal implementation of atomic operations to use ZC
> > > instead of
> > ZR.
> > >
> > > This fix accurately describes the memory constraints for the LL and
> > > SC instructions.  An offset can therefore be used to access a data
> > > item (ie. %lo ()) rather than always having to load the address
> > > into a register.  Tested for mips32r2, mips32r6 and micromips.
> > >
> > > gcc/
> > >
> > >   * config/mips/constraints.md (ZC): Add support for R6 LL/SC
> > >   offsets.
> > >   (ZD): Update to use ISA_HAS_PREF_LL_SC_9BIT.
> > >   * config/mips/mips.h (ISA_HAS_PREFETCH_9BIT): Rename to...
> > >   (ISA_HAS_PREF_LL_SC_9BIT): ... this. New macro.
> > >   * config/mips/sync.md (sync_compare_and_swap): Use ZC
> > >   instead of ZR for the memory operand of LL/SC.
> > >   (compare_and_swap_12, sync_add): Likewise.
> > >   (sync__12, sync_old__12): Likewise.
> > >   (sync_new__12, sync_nand_12): Likewise.
> > >   (sync_old_nand_12, sync_new_nand_12): Likewise.
> > >   (sync_sub, sync_old_add): Likewise.
> > >   (sync_old_sub, sync_new_add): Likewise.
> > >   (sync_new_sub, sync_): Likewise.
> > >   (sync_old_, sync_new_"):
> > > Likewise.
> > >   (sync_nand, sync_old_nand): Likewise.
> > >   (sync_new_nand, sync_lock_test_and_set):
> > > Likewise.
> > >   (test_and_set_12, atomic_compare_and_swap): Likewise.
> > >   (atomic_exchange_llsc, atomic_fetch_add_llsc):
> > > Likewise.
> > >   * doc/md.texi (ZC): Update description.
> > >
> > > OK to commit?
> > >
> > > diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> > > 9dad480..b608b17 100644
> > > --- a/gcc/config/mips/mips.h
> > > +++ b/gcc/config/mips/mips.h
> > > @@ -1089,8 +1089,8 @@ struct mips_cpu_info {
> > > || mips_isa_rev >= 1) \
> > >&& !TARGET_MIPS16)
> > >
> > > -/* ISA has data prefetch with limited 9-bit displacement.  */
> > > -#define ISA_HAS_PREFETCH_9BIT(mips_isa_rev >= 6)
> > > +/* ISA has data prefetch, LL and SC with limited 9-bit displacement.
> > */
> > > +#define ISA_HAS_PREF_LL_SC_9BIT  (mips_isa_rev >= 6)
> > >
> > I'd like to see this described as something more general.  Say:
> > ISA_HAS_9BIT_DISPLACEMENT.   This patch is okay with that fixup.
> 
> I think I'm OK with changing that but it does leave us with a different issue 
> of
> knowing which subset of instructions should check for 9-bit displacement.
> I.e. not all instructions only have a 9-bit displacement.

I'm open to a different name.  Do you have any other suggestions?   Can we just 
say >= R6?

> A GCC 6 thing would be to look over all the ISA_HAS macros and perhaps do
> some general improvement in the framework we have there. I don't know
> exactly what I'd do but something a bit more table based seems sensible.
> 
Sounds like a good idea.


RE: [PATCH,MIPS] Add support for the R6 LSA and DLSA instructions

2015-01-14 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Monday, January 12, 2015 10:35 AM
> To: Moore, Catherine
> Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> Subject: [PATCH,MIPS] Add support for the R6 LSA and DLSA instructions
> 
> This patch adds support for the R6 [D]LSA instructions.  The support has been
> structured to allow MSA (when implemented) to turn on the same
> instructions as they are also added by the MSA ASE.
> 
> I have continued to use the idea of 'ghost' options in the testsuite to 
> indicate
> what features are required rather than arch revisions.
> 
> Thanks,
> Matthew
> 
> gcc/
> 
>   * config/mips/mips.c (mips_rtx_costs): Set costs for LSA/DLSA.
>   (mips_print_operand): Support 'y' to print exact log2 in decimal
>   of a const_int.
>   * config/mips/mips.h (ISA_HAS_LSA): New define.
>   (ISA_HAS_DLSA): Likewise.
>   * config/mips/mips.md (lsa): New define_insn.
>   * config/mips/predicates.md (const_immlsa_operand): New
> predicate.
> 

This patch is fine.


RE: [PATCH,MIPS] Remove all excess parallel constructs

2015-01-14 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Monday, January 12, 2015 11:12 AM
> To: Moore, Catherine
> Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> Subject: [PATCH,MIPS] Remove all excess parallel constructs
> 
>   * config/mips/micromips.md (*swp): Remove explicit parallel.
>   (jraddiusp, *movep): Likewise.
>   * config/mips/mips-dsp.md (add3): Likewise.
>   (mips_add_s_, sub3):
> Likewise.
>   (mips_sub_s_, mips_addsc):
> Likewise.
>   (mips_addwc, mips_absq_s_): Likewise.
>   (mips_precrq_rs_ph_w, mips_precrqu_s_qb_ph): Likewise.
>   (mips_shll_, mips_shll_s_):
> Likewise.
>   (mips_muleu_s_ph_qbl, mips_muleu_s_ph_qbr): Likewise.
>   (mips_mulq_rs_ph, mips_muleq_s_w_phl, mips_muleq_s_w_phr):
> Likewise.
>   (mips_dpaq_s_w_ph, mips_dpsq_s_w_ph, mips_mulsaq_s_w_ph):
> Likewise.
>   (mips_dpaq_sa_l_w, mips_dpsq_sa_l_w, mips_maq_s_w_phl):
> Likewise.
>   (mips_maq_s_w_phr, mips_maq_sa_w_phl, mips_maq_sa_w_phr):
> Likewise.
>   (mips_extr_w, mips_extr_r_w, mips_extr_rs_w): Likewise.
>   (mips_extr_s_h, mips_extp, mips_extpdp, mips_mthlip): Likewise.
>   (mips_wrdsp): Likewise.
>   * config/mips/mips-dspr2.md (mips_absq_s_qb): Remove explicit
>   parallel.
>   (mips_addu_ph, mips_addu_s_ph, mips_cmpgdu_eq_qb): Likewise.
>   (mips_cmpgdu_lt_qb, mips_cmpgdu_le_qb, mulv2hi3): Likewise.
>   (mips_mul_s_ph, mips_mulq_rs_w, mips_mulq_s_ph): Likewise.
>   (mips_mulq_s_w, mips_subu_ph, mips_subu_s_ph): Likewise.
>   (mips_dpaqx_s_w_ph, mips_dpaqx_sa_w_ph): Likewise.
>   (mips_dpsqx_s_w_ph, mips_dpsqx_sa_w_ph): Likewise.
>   * config/mips/mips-fixed.md (usadd3): Remove explicit
> parallel.
>   (ssadd3, ussub3, sssub3, ssmul3):
> Likewise.
>   (ssmaddsqdq4, ssmsubsqdq4): Likewise.

This one is OK, too.


RE: [PATCH,MIPS] Only pass floating-point options to the assembler then

2015-01-19 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Monday, January 19, 2015 5:54 PM
> To: Moore, Catherine
> Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> Subject: RE: [PATCH,MIPS] Only pass floating-point options to the assembler
> then
> 
> Hi Catherine,
> 
> > The new behaviour of the GCC driver passing floating point options
> > like -msoft-float to the assembler is essential for the new o32 ABI
> > extensions but is a change in behaviour. In particular GCC 5 used with
> > binutils 2.24 would require a user to fix any hand-crafted code that
> > made use of floating-point instructions when building for soft-float.
> > This patch limits the new behaviour to a combination of GCC and
> > binutils that both have the new ABI support.
> >
> > This patch along with parts of several previous patches need
> > backporting to GCC 4.9 (and GCC 4.8) to enable use of binutils 2.25
> > with those compilers. The GCC 4.9 patch will be posted shortly.
> 
> I'm not sure if you missed this patch last week or if you are unsure about it?
> Since this is about restoring previous behaviour of MIPS GCC when built
> alongside binutils <= 2.24 I believe this still fits with stage4 criteria.
> 
Hi Matthew,
I didn't miss it, but I wanted to complete some testing with it.  Those tests 
have now completed and this patch is OK.
Sorry for the delay.
Thanks,
Catherine

> >
> > gcc/
> > * config/mips/mips.h (FP_ASM_SPEC): New define.
> > (ASM_SPEC): Remove floating-point options and use FP_ASM_SPEC
> > instead.
> > ---
> >  gcc/config/mips/mips.h | 21 ++---
> >  1 file changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> > 37d4cb4..ed241fa 100644
> > --- a/gcc/config/mips/mips.h
> > +++ b/gcc/config/mips/mips.h
> > @@ -1243,6 +1243,22 @@ struct mips_cpu_info {  %{gcoff*:-mdebug}
> > %{!gcoff*:-no-mdebug}"
> >  #endif
> >
> > +/* FP_ASM_SPEC represents the floating-point options that must be
> > passed
> > +   to the assembler when FPXX support exists.  Prior to that point the
> > +   assembler could accept the options but were not required for
> > +   correctness.  We only add the options when absolutely necessary
> > +   because passing -msoft-float to the assembler will cause it to
> > reject
> > +   all hard-float instructions which may require some user code to be
> > +   updated.  */
> > +
> > +#ifdef HAVE_AS_DOT_MODULE
> > +#define FP_ASM_SPEC "\
> > +%{mhard-float} %{msoft-float} \
> > +%{msingle-float} %{mdouble-float}"
> > +#else
> > +#define FP_ASM_SPEC
> > +#endif
> > +
> >  /* SUBTARGET_ASM_SPEC is always passed to the assembler.  It may be
> > overridden by subtargets.  */
> >
> > @@ -1277,9 +1293,8 @@ struct mips_cpu_info {  %{modd-spreg} %{mno-
> odd-
> > spreg} \  %{mshared} %{mno-shared} \  %{msym32} %{mno-sym32} \ -
> > %{mtune=*} \ -%{mhard-float} %{msoft-float} \ -%{msingle-float}
> > %{mdouble-float} \
> > +%{mtune=*}" \
> > +FP_ASM_SPEC "\
> >  %(subtarget_asm_spec)"
> >
> >  /* Extra switches sometimes passed to the linker.  */
> > --
> > 2.2.1



RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators

2015-01-23 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Friday, January 23, 2015 2:51 PM
> To: Robert Suchanek; gcc-patches@gcc.gnu.org
> Cc: Moore, Catherine
> Subject: RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators
> 
> > 2015-01-23  Robert Suchanek  
> >
> > * config/mips/mips.c (mips_hard_regno_mode_ok_p): Prohibit
> > accumulators
> > for all vector modes.
> 
> This seems like a genuine bug and although it can only be triggered by
> loongson or paired-single support it probably qualifies for fixing.
> My suspicion is that the switch to LRA since GCC 4.9 may be the reason this
> hasn't been noticed before. Reload seemed better in some cases at
> eliminating bad decisions from IRA so this may have simply never made it
> through reload by fluke.
> 
> I'd like Catherine to review too since we are in stage4 without a reproducible
> test case.
> 

The patch looks reasonable, but I'd like to see a test case that fails before 
we agree to include for GCC 5.0. 



RE: [PATCH MIPS RFA] Regression cleanup for nan2008 toolchain

2015-01-26 Thread Moore, Catherine


> -Original Message-
> From: Robert Suchanek [mailto:robert.sucha...@imgtec.com]
> Sent: Monday, January 26, 2015 6:48 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Matthew Fortune; Moore, Catherine
> Subject: [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.
> ---

This patch is OK.



RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators

2015-01-27 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Tuesday, January 27, 2015 7:19 AM
> To: Richard Sandiford
> Cc: Robert Suchanek; gcc-patches@gcc.gnu.org; Moore, Catherine
> Subject: RE: [PATCH RFA MIPS] Prohibit vector modes in accumulators
> 
> Richard Sandiford  writes:
> > Matthew Fortune  writes:
> > >> 2015-01-23  Robert Suchanek  
> > >>
> > >>  * config/mips/mips.c (mips_hard_regno_mode_ok_p): Prohibit
> > >> accumulators
> > >>  for all vector modes.
> > >
> > > This seems like a genuine bug and although it can only be triggered
> > > by loongson or paired-single support it probably qualifies for fixing.
> >
> > Agreed FWIW.  We shouldn't mark something as valid for a mode if even
> > the mode's move pattern can't handle it.
> >
> > I think this kind of thing should go in regardless of development stage.
> 
> Given that it was one of the pre-existing tests that failed I'm happy that we
> are covering this issue. All of these LRA related issues are likely to phase 
> in
> and out with subtle changes to code-gen so I don't think we can always get a
> test case that fails on trunk.
> 
That's true.

> 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


RE: [PATCH MIPS RFA] Regression cleanup for nan2008 toolchain

2015-02-03 Thread Moore, Catherine


> -Original Message-
> From: Robert Suchanek [mailto:robert.sucha...@imgtec.com]
> Sent: Monday, February 02, 2015 11:18 AM
> To: Richard Sandiford
> Cc: gcc-patches@gcc.gnu.org; Matthew Fortune; Moore, Catherine
> Subject: 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" } */
> 

Thanks for the update.  This is OK.
Catherine


RE: [PATCH,WWWDOCS] MIPS changes for GCC 5.0

2015-02-04 Thread Moore, Catherine
Hi Matthew,

I made a few edits.  I removed the markup in the process, so that will need to 
be added back.
See the text at the end.

Thanks,
Catherine

> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Wednesday, February 04, 2015 11:46 AM
> To: Moore, Catherine
> Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> Subject: [PATCH,WWWDOCS] MIPS changes for GCC 5.0
> 
> Hi Catherine,
> 
> I've made a first pass at writing up the MIPS changes for GCC 5.0.
> Could you take a read and see what needs some more work?
> 
> Thanks,
> Matthew
> 
> Index: htdocs/gcc-5/changes.html
> ==
> =
> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-5/changes.html,v
> retrieving revision 1.77
> diff -r1.77 changes.html
> 562a563,614
> > MIPS
> >   
> > MIPS Releases 3 and 5 are now directly supported using -
> mips32r3,
> > -mips64r3, -mips32r5 and -mips64r5 instead of relying on the
> Release
> > 2 options.
> > Support for the Imagination P5600 processor has been added using
> > -march=p5600.
> > 
> > Support for the Cavium Networks Octeon3 processor has been added
> using
> > -march=octeon3.
> > MIPS Release 6 is now supported using -mips32r6 and -
> mips64r6
> > .
> > The previous o32 64-bit floating-point register support has been
> > obsoleted and removed.  This was previously enabled using -
> mfp64
> >  which has been re-purposed for the new ABI extensions
> described
> > below.
> > New o32 ABI extensions have been added to enable software to
> transition
> > away from the original layout of double-precision floating-point 
> > registers.
> > 
> >   The first of these extensions is o32 FPXX which places 
> > restrictions
> >   on code-generation to never access the upper 32-bits of double-
> precision
> >   registers via odd-numbered single-precision registers.  By default the
> >   odd-numbered single-precision registers are not used at all with this
> >   extension.  o32 FPXX code is link compatible with all other o32
> >   double-precision ABI variants and will execute correctly in all 
> > hardware
> >   FPU modes.  Enable o32 FPXX using -mabi=32 -mfpxx for
> >   MIPS II onwards.
> >   The second extension is o32 FP64A which requires 64-bit
> >   floating-point registers and places a mandatory restriction on the 
> > use of
> >   odd-numbered single-precision registers.  o32 FP64A is link compatible
> >   with all other o32 double-precision ABI variants.  Enable o32 FP64A
> >   using -mabi=32 -mfp64 -mno-odd-spreg for MIPS32R2
> onwards.
> >   
> >   Finally, the o32 FP64 extension which also requires 64-bit
> >   floating-point registers but permits the use of all single-precision
> >   registers.  Enable o32 FP64 using -mfp64 for MIPS32R2
> >   onwards.
> > 
> > All new ABI variants can be enabled by default using configure time
> > options --with-fp-32=[32|xx|64] and
> > --with(out)-odd-sp-reg-32.  It is strongly recommended
> that
> > all vendors begin to set o32 FPXX as default ABI to be able to run the
> > generated code on MIPSR5 cores alongside future MIPS SIMD (MSA)
> code and
> > MIPSR6 cores.
> > When using binutils 2.25 GCC will now pass options like
> > -msoft-float and -msingle-float to the
> assembler.
> > This change can affect inline assembly code that is built as soft-float 
> > but
> > contains hard-float instructions.  In such cases the code must be
> amended
> > to use appropriate .set directives to override the global
> > assembler options.
> >   
> >

MIPS Releases 3 and 5 are now directly supported.  Use the command-line options
-mips32r3, -mips64r3, -mips32r5 and -mips64r5 to enable code-generation for
these processors.

Support for the Imagination P5600 processor is now supported through
use of the -march=p5600 command-line option.

The Cavium Octeon3 processor is now supported through the use of the
command-line option -march=octeon3.

MIPS Release 6 is now supported through the use of the -mips32r6 and -mips64r6
command-line options.

The o32 ABI has been modified and extended.  The o32 64-bit floating-point
register support is now obsolete and has been removed.  It has been replaced by
three ABI extensions FPXX, FP64A, and FP64.  The meaning of the -mfp64
command-line option has been changed and it is now used to enable the ABI
extensions.

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

2015-02-06 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Thursday, February 05, 2015 3:52 PM
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> > 
> I've put your patch inline below and switched to plain text. I suspect your
> post was bounced by gcc-patches.
> 
> I'm OK with this change but I'd like Catherine to comment before committing.
> It seems a shame to duplicate the block of code but it is probably just as 
> ugly
> to define a macro for the la/dla instruction.

The patch itself is OK, although I agree with other's comments about the 
unfortunate duplication of code.
Petar,  would you please add your testcase to the gcc testsuite and repost the 
patch.
Thanks,
Catherine

> 
> For future reference, it is best not to include changelog content in a patch
> but instead just paste into the email. Also the ChangeLog you need for this
> change is gcc/ChangeLog (as confusing as that may be at first).
> 
> Thanks,
> Matthew
> 
> > From: Petar Jovanovic [mailto:petar.jovano...@rt-rk.com]
> > Sent: 05 February 2015 19:28
> > To: gcc-patches@gcc.gnu.org; 'Maciej W. Rozycki'; Matthew Fortune
> > Subject: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> >
> > v2:
> > - add ChangeLog entry
> > - use DLA instead of LA for n64
> >
> > PTAL. Thanks.
> >
> > Regards,
> > Petar
> 
> ---
>  ChangeLog  |  5 +
>  gcc/config/mips/mips.h | 23 +++
>  2 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 5c61c66..3a15f4a 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2015-02-05  Petar Jovanovic  
> +
> + * config/mips/mips.h (CRT_CALL_STATIC_FUNCTION): Fix the macro
> to use
> + la/jalr instead of jal.
> +
>  2015-02-02  Janis Johnson  
> 
>   * MAINTAINERS (Various Maintainers: testsuite): Remove myself.
> diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> ec69ed5..4bd83f5 100644
> --- a/gcc/config/mips/mips.h
> +++ b/gcc/config/mips/mips.h
> @@ -3034,11 +3034,11 @@ while (0)
>   nop\n\
>  1:   .cpload $31\n\
>   .set reorder\n\
> - jal " USER_LABEL_PREFIX #FUNC "\n\
> + la $25, " USER_LABEL_PREFIX #FUNC "\n\
> + jalr $25\n\
>   .set pop\n\
>   " TEXT_SECTION_ASM_OP);
> -#elif ((defined _ABIN32 && _MIPS_SIM == _ABIN32) \
> -   || (defined _ABI64 && _MIPS_SIM == _ABI64))
> +#elif (defined _ABIN32 && _MIPS_SIM == _ABIN32)
>  #define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)   \
> asm (SECTION_OP "\n\
>   .set push\n\
> @@ -3048,7 +3048,22 @@ while (0)
>   nop\n\
>  1:   .set reorder\n\
>   .cpsetup $31, $2, 1b\n\
> - jal " USER_LABEL_PREFIX #FUNC "\n\
> + la $25, " USER_LABEL_PREFIX #FUNC "\n\
> + jalr $25\n\
> + .set pop\n\
> + " TEXT_SECTION_ASM_OP);
> +#elif (defined _ABI64 && _MIPS_SIM == _ABI64)
> +#define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)   \
> +   asm (SECTION_OP "\n\
> + .set push\n\
> + .set nomips16\n\
> + .set noreorder\n\
> + bal 1f\n\
> + nop\n\
> +1:   .set reorder\n\
> + .cpsetup $31, $2, 1b\n\
> + dla $25, " USER_LABEL_PREFIX #FUNC "\n\
> + jalr $25\n\
>   .set pop\n\
>   " TEXT_SECTION_ASM_OP);
>  #endif
> --
> 1.8.2.1



RE: [PATCH,WWWDOCS] MIPS changes for GCC 5.0

2015-02-06 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Thursday, February 05, 2015 5:24 AM
> To: Moore, Catherine
> Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> Subject: RE: [PATCH,WWWDOCS] MIPS changes for GCC 5.0
> 
> Moore, Catherine  writes:
> > Hi Matthew,
> >
> > I made a few edits.  I removed the markup in the process, so that will
> > need to be added back.
> > See the text at the end.
> 
> Thanks Catherine. Good call to remove the markup while reviewing. I've
> done one more pass on this to have the same phrasing used where similar
> points are being made. I also added a comment about link compatibility for
> FP64.  Updated text is at the end.
> 
This looks good now.

> >
> > > -Original Message-
> > > From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> > > Sent: Wednesday, February 04, 2015 11:46 AM
> > > To: Moore, Catherine
> > > Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> > > Subject: [PATCH,WWWDOCS] MIPS changes for GCC 5.0
> > >
> > > Hi Catherine,
> > >
> > > I've made a first pass at writing up the MIPS changes for GCC 5.0.
> > > Could you take a read and see what needs some more work?
> > >
> > > Thanks,
> > > Matthew
> > >
> > > Index: htdocs/gcc-5/changes.html
> > >
> ==
> > > =
> > > RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-5/changes.html,v
> > > retrieving revision 1.77
> > > diff -r1.77 changes.html
> > > 562a563,614
> > > > MIPS
> > > >   
> > > > MIPS Releases 3 and 5 are now directly supported using
> > > > -
> > > mips32r3,
> > > > -mips64r3, -mips32r5 and -mips64r5 instead of relying
> > > > on the
> > > Release
> > > > 2 options.
> > > > Support for the Imagination P5600 processor has been added
> > using
> > > > -march=p5600.
> > > > 
> > > > Support for the Cavium Networks Octeon3 processor has been
> > > > added
> > > using
> > > > -march=octeon3.
> > > > MIPS Release 6 is now supported using -mips32r6 and
> > > > -
> > > mips64r6
> > > > .
> > > > The previous o32 64-bit floating-point register support
> > > > has
> > been
> > > > obsoleted and removed.  This was previously enabled using
> > > > -
> > > mfp64
> > > >  which has been re-purposed for the new ABI extensions
> > > described
> > > > below.
> > > > New o32 ABI extensions have been added to enable software
> > > > to
> > > transition
> > > > away from the original layout of double-precision
> > > > floating-point
> > registers.
> > > > 
> > > >   The first of these extensions is o32 FPXX which places
> > restrictions
> > > >   on code-generation to never access the upper 32-bits of
> > > > double-
> > > precision
> > > >   registers via odd-numbered single-precision registers.  By
> > default the
> > > >   odd-numbered single-precision registers are not used at all
> > with this
> > > >   extension.  o32 FPXX code is link compatible with all other
> > o32
> > > >   double-precision ABI variants and will execute correctly in
> > all hardware
> > > >   FPU modes.  Enable o32 FPXX using -mabi=32
> > > > -mfpxx
> > for
> > > >   MIPS II onwards.
> > > >   The second extension is o32 FP64A which requires 64-bit
> > > >   floating-point registers and places a mandatory restriction
> > > > on
> > the use of
> > > >   odd-numbered single-precision registers.  o32 FP64A is link
> > compatible
> > > >   with all other o32 double-precision ABI variants.  Enable
> > > > o32
> > FP64A
> > > >   using -mabi=32 -mfp64 -mno-odd-spreg for
> > > > MIPS32R2
> > > onwards.
> > > >   
> > > >   Finally, the o32 FP64 extension which also requires 64-bit
> > > >   floating-point registers but permits the use of all single-
> > precision
> > > >   registers.  Enable o32 FP64 using -mfp64 for
> > MIPS32R2
> > &g

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

2015-02-12 Thread Moore, Catherine


> -Original Message-
> From: Petar Jovanovic [mailto:petar.jovano...@rt-rk.com]
> Sent: Thursday, February 12, 2015 7:28 PM
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> 
> -Original Message-----
> From: Moore, Catherine [mailto:catherine_mo...@mentor.com]
> Sent: Friday, February 6, 2015 4:13 PM
> To: Matthew Fortune; Petar Jovanovic; gcc-patches@gcc.gnu.org; 'Maciej W.
> Rozycki'
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> 
> 
> > Petar,  would you please add your testcase to the gcc testsuite and repost
> the patch.
> 
> Hi Catherine,
> 
> Sure, I can repost the patch and add the test case I used.
> My concern - and I would appreciate feedback from you and others on it - is
> that the test I originally used to report [1] the problem creates a large
> executable (380 MB).
> 
> 
> The test also takes time to execute. Obviously, I can make it slightly more
> complex and make execute time short. Another path is to introduce and use
> options such as "Wl,-Ttext-segment" to make its size smaller. Any other
> ideas?
> 
Hi Petar,

There isn't any need to execute a large testcase.  Instead, try adding a short 
version of your test to the directory gcc/testsuite/gcc.target/mips.
There are examples of other tests there they check for specific assembler 
sequences by using scan-assembler.   See umips-swp-1.c (and others) for a 
specific example of how to do this.
Let me know if you need additional help.
Thanks,
Catherine




RE: [Patch] Add MIPS flag to avoid use of ldc1/sdc1/ldxc1/sdxc1

2014-10-27 Thread Moore, Catherine


> -Original Message-
> From: Andrew Pinski [mailto:pins...@gmail.com]
> Sent: Monday, October 27, 2014 6:32 PM
> To: Steve Ellcey
> Cc: Moore, Catherine; Matthew Fortune; GCC Patches
> Subject: Re: [Patch] Add MIPS flag to avoid use of ldc1/sdc1/ldxc1/sdxc1
> 
> On Mon, Oct 27, 2014 at 3:23 PM, Steve Ellcey  wrote:
> >
> > There are some MIPS patches that have been applied to the Google
> > Android GCC tree but not been submitted to FSF GCC.  I would like to get
> those patches
> > checked in if possible.   The first one is to add a new MIPS flag to turn
> > off the use of ldc1/sdc1/ldxc1/sdxc1.  Google Earth has a custom
> > allocator that does not guarantee 8 byte alignment of the allocated
> > memory, therefore using these instructions (that require 8 byte
> > alignment) could cause runtime failures.  By default, these
> > instructions will continue to be used, the flag is there to allow their use 
> > to
> be turned off if desired.
> 
> That sounds like a bug in google earth's allocator if we follow either C or 
> C++
> standards when it comes to malloc/operator new.
> Both have to be align enough for the type which is at least that size that is
> being allocated.  So it needs to be 8byte aligned if allocating a 8 byte 
> object.
> 

  Andrew,

  Do you have an objection to allowing an option to disable these instructions 
(despite the reason for wanting to do so)?

  Thanks,
  Catherine
  
> 
> >
> > Tested on mips-mti-linux-gnu.
> >
> > OK to checkin?
> >
> > Steve Ellcey
> > sell...@imgtec.com
> >
> >
> >
> > 2014-10-27  Steve Ellcey  
> >
> > * config/mips/mips.h (ISA_HAS_LDC1_SDC1): Check
> TARGET_LDC1_SDC1.
> > * config/mips/mips.md (*_ > * config/mips/mips.md (*_ > * config/mips/mips.opt (mldc1-sdc1): New.
> >
> >
> > diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> > c7b998b..b9caff7 100644
> > --- a/gcc/config/mips/mips.h
> > +++ b/gcc/config/mips/mips.h
> > @@ -892,7 +892,8 @@ struct mips_cpu_info {
> >  /* ISA has LDC1 and SDC1.  */
> >  #define ISA_HAS_LDC1_SDC1  (!ISA_MIPS1 \
> >  && !TARGET_MIPS5900\
> > -&& !TARGET_MIPS16)
> > +&& !TARGET_MIPS16  \
> > +&& TARGET_LDC1_SDC1)
> >
> >  /* ISA has the mips4 FP condition code instructions: FP-compare to CC,
> > branch on CC, and move (both FP and non-FP) on CC.  */ diff --git
> > a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index
> > d47bb78..77f3356 100644
> > --- a/gcc/config/mips/mips.md
> > +++ b/gcc/config/mips/mips.md
> > @@ -4573,7 +4573,7 @@
> >[(set (match_operand:ANYF 0 "register_operand" "=f")
> > (mem:ANYF (plus:P (match_operand:P 1 "register_operand" "d")
> >   (match_operand:P 2 "register_operand"
> > "d"]
> > -  "ISA_HAS_LXC1_SXC1"
> > +  "ISA_HAS_LXC1_SXC1 && (mode == SFmode ||
> TARGET_LDC1_SDC1)"
> >"\t%0,%1(%2)"
> >[(set_attr "type" "fpidxload")
> > (set_attr "mode" "")]) @@ -4582,7 +4582,7 @@
> >[(set (mem:ANYF (plus:P (match_operand:P 1 "register_operand" "d")
> >   (match_operand:P 2 "register_operand" "d")))
> > (match_operand:ANYF 0 "register_operand" "f"))]
> > -  "ISA_HAS_LXC1_SXC1"
> > +  "ISA_HAS_LXC1_SXC1 && (mode == SFmode ||
> TARGET_LDC1_SDC1)"
> >"\t%0,%1(%2)"
> >[(set_attr "type" "fpidxstore")
> > (set_attr "mode" "")]) diff --git
> > a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt index
> > dca4f80..7b75fc9 100644
> > --- a/gcc/config/mips/mips.opt
> > +++ b/gcc/config/mips/mips.opt
> > @@ -267,6 +267,10 @@ mips3d
> >  Target Report RejectNegative Var(TARGET_MIPS3D)  Use MIPS-3D
> > instructions
> >
> > +mldc1-sdc1
> > +Target Report Var(TARGET_LDC1_SDC1) Init(1) Use ldc1/ldxc1 and
> > +sdc1/sdxc1 instruction
> > +
> >  mllsc
> >  Target Report Mask(LLSC)
> >  Use ll, sc and sync instructions


RE: [Patch] Add MIPS flag to avoid use of ldc1/sdc1/ldxc1/sdxc1

2014-10-27 Thread Moore, Catherine


> -Original Message-
> From: Andrew Pinski [mailto:pins...@gmail.com]
> Sent: Monday, October 27, 2014 6:41 PM
> To: Moore, Catherine
> Cc: Steve Ellcey; Matthew Fortune; GCC Patches
> Subject: Re: [Patch] Add MIPS flag to avoid use of ldc1/sdc1/ldxc1/sdxc1
> 
> On Mon, Oct 27, 2014 at 3:35 PM, Moore, Catherine
>  wrote:
> >
> >
> >> -Original Message-
> >> From: Andrew Pinski [mailto:pins...@gmail.com]
> >> Sent: Monday, October 27, 2014 6:32 PM
> >> To: Steve Ellcey
> >> Cc: Moore, Catherine; Matthew Fortune; GCC Patches
> >> Subject: Re: [Patch] Add MIPS flag to avoid use of
> >> ldc1/sdc1/ldxc1/sdxc1
> >>
> >> On Mon, Oct 27, 2014 at 3:23 PM, Steve Ellcey 
> wrote:
> >> >
> >> > There are some MIPS patches that have been applied to the Google
> >> > Android GCC tree but not been submitted to FSF GCC.  I would like
> >> > to get
> >> those patches
> >> > checked in if possible.   The first one is to add a new MIPS flag to turn
> >> > off the use of ldc1/sdc1/ldxc1/sdxc1.  Google Earth has a custom
> >> > allocator that does not guarantee 8 byte alignment of the allocated
> >> > memory, therefore using these instructions (that require 8 byte
> >> > alignment) could cause runtime failures.  By default, these
> >> > instructions will continue to be used, the flag is there to allow
> >> > their use to
> >> be turned off if desired.
> >>
> >> That sounds like a bug in google earth's allocator if we follow
> >> either C or C++ standards when it comes to malloc/operator new.
> >> Both have to be align enough for the type which is at least that size
> >> that is being allocated.  So it needs to be 8byte aligned if allocating a 
> >> 8 byte
> object.
> >>
> >
> >   Andrew,
> >
> >   Do you have an objection to allowing an option to disable these
> instructions (despite the reason for wanting to do so)?
> 
> Yes this seems like a bad workaround for broken code.
> 
Well, we work around broken hardware all the time.  What would you suggest that 
Steve do instead?
Should we tell him to add an -mfix-google-earth option?All kidding aside, I 
wouldn't mind a viable suggestion.




> >
> >>
> >> >
> >> > Tested on mips-mti-linux-gnu.
> >> >
> >> > OK to checkin?
> >> >
> >> > Steve Ellcey
> >> > sell...@imgtec.com
> >> >
> >> >
> >> >
> >> > 2014-10-27  Steve Ellcey  
> >> >
> >> > * config/mips/mips.h (ISA_HAS_LDC1_SDC1): Check
> >> TARGET_LDC1_SDC1.
> >> > * config/mips/mips.md (*_ >> > * config/mips/mips.md (*_ >> > * config/mips/mips.opt (mldc1-sdc1): New.
> >> >
> >> >
> >> > diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> >> > c7b998b..b9caff7 100644
> >> > --- a/gcc/config/mips/mips.h
> >> > +++ b/gcc/config/mips/mips.h
> >> > @@ -892,7 +892,8 @@ struct mips_cpu_info {
> >> >  /* ISA has LDC1 and SDC1.  */
> >> >  #define ISA_HAS_LDC1_SDC1  (!ISA_MIPS1 \
> >> >  && !TARGET_MIPS5900\
> >> > -&& !TARGET_MIPS16)
> >> > +&& !TARGET_MIPS16  \
> >> > +&& TARGET_LDC1_SDC1)
> >> >
> >> >  /* ISA has the mips4 FP condition code instructions: FP-compare to CC,
> >> > branch on CC, and move (both FP and non-FP) on CC.  */ diff
> >> > --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index
> >> > d47bb78..77f3356 100644
> >> > --- a/gcc/config/mips/mips.md
> >> > +++ b/gcc/config/mips/mips.md
> >> > @@ -4573,7 +4573,7 @@
> >> >[(set (match_operand:ANYF 0 "register_operand" "=f")
> >> > (mem:ANYF (plus:P (match_operand:P 1 "register_operand" "d")
> >> >   (match_operand:P 2 "register_operand"
> >> > "d"]
> >> > -  "ISA_HAS_LXC1_SXC1"
> >> > +  "ISA_HAS_LXC1_SXC1 && (mode == SFmode ||
> >> TARGET_LDC1_SDC1)"
> >> >"\t%0,%1(%2)"
> >> >[(set_attr "type" "fpidxload")
> >> > (set_attr "mode" "")]) @@ -4582,7 +4582,7 @@
> >> >[(set (mem:ANYF (plus:P (match_operand:P 1 "register_operand" "d")
> >> >   (match_operand:P 2 "register_operand" "d")))
> >> > (match_operand:ANYF 0 "register_operand" "f"))]
> >> > -  "ISA_HAS_LXC1_SXC1"
> >> > +  "ISA_HAS_LXC1_SXC1 && (mode == SFmode ||
> >> TARGET_LDC1_SDC1)"
> >> >"\t%0,%1(%2)"
> >> >[(set_attr "type" "fpidxstore")
> >> > (set_attr "mode" "")]) diff --git
> >> > a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt index
> >> > dca4f80..7b75fc9 100644
> >> > --- a/gcc/config/mips/mips.opt
> >> > +++ b/gcc/config/mips/mips.opt
> >> > @@ -267,6 +267,10 @@ mips3d
> >> >  Target Report RejectNegative Var(TARGET_MIPS3D)  Use MIPS-3D
> >> > instructions
> >> >
> >> > +mldc1-sdc1
> >> > +Target Report Var(TARGET_LDC1_SDC1) Init(1) Use ldc1/ldxc1 and
> >> > +sdc1/sdxc1 instruction
> >> > +
> >> >  mllsc
> >> >  Target Report Mask(LLSC)
> >> >  Use ll, sc and sync instructions


RE: [PATCHv2][MIPS] Implement O32 ABI extensions (GCC)

2014-10-27 Thread Moore, Catherine
Hi Matthew,
Review comments are attached.  I will tackle the R6 patch next.
Thanks,
Catherine

> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Friday, August 22, 2014 5:43 AM
> To: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org); Eric Christopher
> (echri...@gmail.com)
> Cc: Moore, Catherine; Richard Sandiford; Rich Fuhler; Rozycki, Maciej; Myers,
> Joseph
> Subject: [PATCHv2][MIPS] Implement O32 ABI extensions (GCC)
> 
> Updated patch covering all comments from the previous thread:
> https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00401.html
> 
> This patch has merged together the odd-spreg work with the other ABI work
> as these two features are now inseparable due to the inclusion of a 4th FP
> ABI variant called FP64A. The wiki page describing these extensions has been
> updated and the patch is consistent with the features.
> 
> https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-
> _FR0_and_FR1_Interlinking
> 
> It is worth noting that LLVM 3.5 will include all ABI extensions described in 
> the
> wiki page with consistent options and behaviour.
> 
> The vast majority of this patch has been reviewed in detail already and
> testing has been ongoing for some time within teams at Imagination.
> This code has also been released for inclusion in the next Android NDK.
> 
> I have addressed one of the final concerns from Richard and Maciej regarding
> an inconsistency between the prologue FP callee-save and fixed-regs. I
> opted to resolve this in a very focussed manner to just address the impact of
> fixed-regs rather than the more general issue of occasionally saving more
> state than absolutely required. If there is a desire to improve this further
> then I am very keen to leave that to a future patch.
> 
> This work has been tested for both bare metal and linux based targets.
> There are no regressions.
> 
> This patch is dependent on two patches which are awaiting approval:
> 
> "Add target hook to override DWARF2 frame register size
> https://gcc.gnu.org/ml/gcc-patches/2014-08/msg01748.html
> 
> "Do not reload unallocated FP_REGS pseudos via GR_REGS"
> https://gcc.gnu.org/ml/gcc-patches/2014-08/msg01745.html
> 
> Regards,
> Matthew
> 
> 2014-07-31  Matthew Fortune  
> 
> gcc/
>   * common/config/mips/mips-common.c (mips_handle_option):
> Ensure
>   that -mfp32, -mfp64 disable -mfpxx and -mfpxx disables -mfp64.
>   * config.gcc (--with-fp-32): New option.
>   (--with-odd-spreg-32): Likewise.
>   * config.in (HAVE_AS_MODULE): New config define.
>   * config/mips/mips-protos.h
>   (mips_secondary_memory_needed): New prototype.
>   (mips_hard_regno_caller_save_mode): Likewise.
>   * config/mips/mips.c (mips_get_reg_raw_mode): New static
> prototype.
>   (mips_get_arg_info): Assert that V2SFmode is only handled specially
>   with TARGET_PAIRED_SINGLE_FLOAT.
>   (mips_return_mode_in_fpr_p): Likewise.
>   (mips16_call_stub_mode_suffix): Likewise.
>   (mips_get_reg_raw_mode): New static function.
>   (mips_return_fpr_pair): O32 return values span two registers.
>   (mips16_build_call_stub): Likewise.
>   (mips_function_value_regno_p): Support both FP return registers.
>   (mips_output_64bit_xfer): Use mthc1 whenever
> TARGET_HAS_MXHC1.  Add
>   specific cases for TARGET_FPXX to move via memory.
>   (mips_dwarf_register_span): For TARGET_FPXX pretend that modes
> larger
>   than UNITS_PER_FPREG 'span' one register.
>   (mips_dwarf_frame_reg_mode): New static function.
>   (mips_file_start): Switch to using .module instead of .gnu_attribute.
>   No longer support FP ABI 4 (-mips32r2 -mfp64), replace with FP ABI 6.
>   Add FP ABI 5 (-mfpxx) and FP ABI 7 (-mfp64 -mno-odd-spreg).
>   (mips_save_reg, mips_restore_reg): Always represent DFmode
> frame
>   slots with two CFI directives even for O32 FP64.
>   (mips_for_each_saved_gpr_and_fpr): Account for fixed_regs when
>   saving/restoring callee-saved registers.
>   (mips_hard_regno_mode_ok_p): Implement O32 FP64A extension.
>   (mips_secondary_memory_needed): New function.
>   (mips_option_override): ABI check for TARGET_FLOATXX.  Disable
>   odd-numbered single-precision registers when using
> TARGET_FLOATXX.
>   Implement -modd-spreg and defaults.
>   (mips_conditional_register_usage): Redefine O32 FP64 to match O32
> FP32
>   callee-saved behaviour.
>   (mips_hard_regno_caller_save_mode): Implement.
>   (TARGET_GET_RAW_RESULT_MODE): Define target hook.
>   (TARGET_GET_RAW_ARG_MODE): Define target hook.
>   (TARGET_DWARF_FRAME_REG_MO

RE: [PATCHv2][MIPS] Implement O32 ABI extensions (GCC)

2014-10-29 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Tuesday, October 28, 2014 1:13 PM
> To: Moore, Catherine; 'gcc-patches@gcc.gnu.org' (gcc-
> patc...@gcc.gnu.org); Eric Christopher (echri...@gmail.com)
> Cc: Richard Sandiford; Rich Fuhler; Rozycki, Maciej; Myers, Joseph
> Subject: RE: [PATCHv2][MIPS] Implement O32 ABI extensions (GCC)
> 
> Moore, Catherine  writes:
> > Review comments are attached.  I will tackle the R6 patch next.
> > Thanks,
> > Catherine
> 
> Thanks Catherine. Depending on whether the fix to the following hunk
> needs discussion I'll make the changes and commit.
> 
> >> static bool
> >> mips_function_value_regno_p (const unsigned int regno) {
> >>   if (regno == GP_RETURN
> >>   || regno == FP_RETURN
> >>+  || regno == FP_RETURN + 2
> >>   || (LONG_DOUBLE_TYPE_SIZE == 128
> >>  && FP_RETURN != GP_RETURN
> >>  && regno == FP_RETURN + 2))
> >> return true;
> >>
> >  Is this right? I'm  not following the intent of this modification.
> 
> The intent is to cover the complex return case where both FP_RETURN and
> FP_RETURN+2 are used to return the real and imaginary parts of _Complex
> float. The hunk which follows then deals with the _Complex double case
> where the corresponding odd numbered registers are also used for FP32.
> What I noticed when you first pointed me at this code on IRC was that the
> new condition leads to the pre-existing LONG_DOUBLE_TYPE_SIZE condition
> being redundant so should be removed I think. I just want to re-read how
> these functions are used to reassure myself this is all correct.

Okay, will you please add a comment here and for the next hunk?

> 
> I'll also check on whether o32 or O32 is the consistent name and change
> appropriately.
> 
> For the R6 patch. I will be rebasing and updating that patch following
> submission of FPXX but the only significant fix it needs relates to allowing 
> the
> use of DSP with R6. That will affect the conditions on a few patterns and
> some of the expand code. A general read through the current patch on the
> list wouldn't be wasted though.
> 
Thanks,
Catherine



RE: [Patch] MIPS configuration patch

2014-10-30 Thread Moore, Catherine


> -Original Message-
> From: Steve Ellcey [mailto:sell...@imgtec.com]
> Sent: Thursday, October 23, 2014 4:37 PM
> To: matthew.fort...@imgtec.com; Moore, Catherine; gcc-
> patc...@gcc.gnu.org
> Subject: [Patch] MIPS configuration patch
> 
> Here is another patch to reduce the differences between the 32 bit MIPS and
> 64 bit MIPS configuration.  This patch combines the two case entries in
> config.gcc into one entry so the header file list and other settings don't 
> have
> to be handled twice.
> 
> This patch should not change the build for any MIPS target except that,
> before this patch, the gnu_ld and gas variables were explicitly set to yes for
> MIPS64 targets but not for MIPS32 targets.  I don't know why this difference
> exists but it shouldn't matter for builds that are on linux because the
> variables were getting set to 'yes' anyway when using the GNU linker and
> assembler.  I don't think anyone builds a linux target that does not use 
> these.
> I chose to put the settings in (matching MIPS64) but I don't think removing
> them would cause any problems if we want to do that.
> 

Hi Steve,
The gnu_ld and gas settings can be removed altogether.  They are set earlier 
for all linux-targets:

*-*-linux* | frv-*-*linux* | *-*-kfreebsd*-gnu | *-*-knetbsd*-gnu | *-*-gnu* | 
*-*-kopensolaris*-gnu)
  extra_options="$extra_options gnu-user.opt"
  gas=yes
  gnu_ld=yes
  case ${enable_threads} in
"" | yes | posix) thread_file='posix' ;;
  Esac

This patch is OK, with that change.
Thanks,
Catherine


> 
> 
> 2014-10-23  Steve Ellcey  
> 
>   * config.gcc (mips*-*-linux*): Combine 32 and 64 bit cases.
> 
> 
> diff --git a/gcc/config.gcc b/gcc/config.gcc index 626..8bc59bf 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -1945,41 +1945,46 @@ mips*-mti-linux*)
>   gnu_ld=yes
>   gas=yes
>   ;;
> -mips64*-*-linux* | mipsisa64*-*-linux*)
> +mips*-*-linux*)  # Linux MIPS, either endian.
>   tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-
> stdint.h ${tm_file} mips/gnu-user.h mips/linux.h mips/linux-common.h"
>   extra_options="${extra_options} linux-android.opt"
> - tmake_file="${tmake_file} mips/t-linux64"
> - tm_defines="${tm_defines} MIPS_ABI_DEFAULT=ABI_N32"
>   case ${target} in
> + mipsisa32r2*)
> + tm_defines="${tm_defines}
> MIPS_ISA_DEFAULT=33"
> + ;;
> + mipsisa32*)
> + tm_defines="${tm_defines}
> MIPS_ISA_DEFAULT=32"
> + ;;
>   mips64el-st-linux-gnu)
> + tm_defines="${tm_defines}
> MIPS_ABI_DEFAULT=ABI_N32"
>   tm_file="${tm_file} mips/st.h"
>   tmake_file="${tmake_file} mips/t-st"
> + enable_mips_multilibs="yes"
>   ;;
>   mips64octeon*-*-linux*)
> + tm_defines="${tm_defines}
> MIPS_ABI_DEFAULT=ABI_N32"
>   tm_defines="${tm_defines}
> MIPS_CPU_STRING_DEFAULT=\\\"octeon\\\""
>   target_cpu_default=MASK_SOFT_FLOAT_ABI
> + enable_mips_multilibs="yes"
>   ;;
>   mipsisa64r2*-*-linux*)
> + tm_defines="${tm_defines}
> MIPS_ABI_DEFAULT=ABI_N32"
>   tm_defines="${tm_defines}
> MIPS_ISA_DEFAULT=65"
> + enable_mips_multilibs="yes"
> + ;;
> + mips64*-*-linux* | mipsisa64*-*-linux*)
> + tm_defines="${tm_defines}
> MIPS_ABI_DEFAULT=ABI_N32"
> + enable_mips_multilibs="yes"
>   ;;
>   esac
> - gnu_ld=yes
> - gas=yes
> - ;;
> -mips*-*-linux*)  # Linux MIPS, either endian.
> - tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-
> stdint.h ${tm_file} mips/gnu-user.h mips/linux.h"
> - extra_options="${extra_options} linux-android.opt"
>   if test x$enable_targets = xall; then
> + enable_mips_multilibs="yes"
> + fi
> + if test x$enable_mips_multilibs = xyes; then
>   tmake_file="${tmake_file} mips/t-linux64"
>   fi
> - tm_file="${tm_file} mips/linux-common.h"
> - case ${target} in
> -mipsisa32r2*)
> - tm_defines="${tm_defines} MIPS_ISA_DEFAULT=33"
> -;;
> -mipsisa32*)
> - tm_defines="${tm_defines} MIPS_ISA_DEFAULT=32"
> -esac
> + gnu_ld=yes
> + gas=yes
>   ;;
>  mips*-mti-elf*)
>   tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/n32-elf.h
> mips/sde.h mips/mti-elf.h"


RE: [Patch, MIPS] Add Octeon3 support

2014-10-30 Thread Moore, Catherine


> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> Sent: Thursday, October 30, 2014 2:59 AM
> Subject: Re: [Patch, MIPS] Add Octeon3 support
> 
> Hi,
> 
> Thanks for the review and your comments.
> 
> >> Is it intentional that you have not updated driver-native.c to detect
> >> an Octeon 3 CPU?
> 
> We have not yet looked into that part yet and will be looking at it later.
> 
> >> Could you confirm what testing the patch has had?
> 
> Run the regression in build folder of gcc using make-check.
> The compilation results are good.
> Also we have been using the octeon3 patches in cavium toolchain from
> sometime without any issues.
> 

Hi Naveen,

This patch looks good except that is missing testcases.  Would you please add 
some testcases and resubmit your patch?
Thanks,
Catherine



RE: [Patch, MIPS] Add Octeon3 support

2014-10-31 Thread Moore, Catherine


> -Original Message-
> From: Hurugalawadi, Naveen
> [mailto:naveen.hurugalaw...@caviumnetworks.com]
> Sent: Friday, October 31, 2014 2:50 AM
> To: Moore, Catherine; Matthew Fortune; Myers, Joseph
> Cc: gcc-patches@gcc.gnu.org; Pinski, Andrew
> Subject: Re: [Patch, MIPS] Add Octeon3 support
> 
> Hi Catherine,
> 
> >> Would you please add some testcases and resubmit your patch?
> 
> Thanks for the review and suggestions.
> Added the testcase "gcc.target/mips/octeon3-pipe-1.c"
> 
> Please review the modified patch and let us know if its good.

This looks OK to commit.
Catherine

> 
> 2014-10-31  Andrew Pinski  
> 
> * config/mips/mips-cpus.def (octeon3): New cpu.
> * config/mips/mips.c (mips_rtx_cost_data): Add octeon3.
> (mips_print_operand ): Fix a bug as the mode
> of the comparison no longer matches mode of the operands.
> (mips_issue_rate): Handle PROCESSOR_OCTEON3.
> * config/mips/mips.h (TARGET_OCTEON):  Add Octeon3.
> (TARGET_OCTEON2): Likewise.
> (TUNE_OCTEON): Add Octeon3.
> * config/mips/mips.md (processor): Add octeon3.
> * config/mips/octeon.md (octeon_fpu): New automaton and cpu_unit.
> (octeon_arith): Add octeon3.
> (octeon_condmove): Remove.
> (octeon_condmove_o1): New reservation.
> (octeon_condmove_o2): New reservation.
> (octeon_condmove_o3_int_on_cc): New reservation.
> (octeon_load_o2): Add octeon3.
> (octeon_cop_o2): Likewise.
> (octeon_store): Likewise.
> (octeon_brj_o2): Likewise.
> (octeon_imul3_o2): Likewise.
> (octeon_imul_o2): Likewise.
> (octeon_mfhilo_o2): Likewise.
> (octeon_imadd_o2): Likewise.
> (octeon_idiv_o2_si): Likewise.
> (octeon_idiv_o2_di): Likewise.
> (octeon_fpu): Add to the automaton.
> (octeon_fpu): New cpu unit.
> (octeon_condmove_o2): Check for non floating point modes.
> (octeon_load_o2): Add prefetchx.
> (octeon_cop_o2): Don't check for octeon3.
> (octeon3_faddsubcvt): New reservation.
> (octeon3_fmul): Likewise.
> (octeon3_fmadd): Likewise.
> (octeon3_div_sf): Likewise.
> (octeon3_div_df): Likewise.
> (octeon3_sqrt_sf): Likewise.
> (octeon3_sqrt_df): Likewise.
> (octeon3_rsqrt_sf): Likewise.
> (octeon3_rsqrt_df): Likewise.
> (octeon3_fabsnegmov): Likewise.
> (octeon_fcond): Likewise.
> (octeon_fcondmov): Likewise.
> (octeon_fpmtc1): Likewise.
> (octeon_fpmfc1): Likewise.
> (octeon_fpload): Likewise.
> (octeon_fpstore): Likewise.
> * config/mips/mips-tables.opt: Regenerate.
> * doc/invoke.texi (-march=@var{arch}): Add octeon3.
> 
> 2014-10-31  Naveen H.S  
> 
> * gcc.target/mips/octeon3-pipe-1.c: New test.


RE: [PATCHv4][MIPS] Implement O32 ABI extensions (GCC)

2014-11-12 Thread Moore, Catherine
Hi Matthew,

> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Thursday, November 06, 2014 12:11 PM
> To: Moore, Catherine; 'gcc-patches@gcc.gnu.org' (gcc-
> patc...@gcc.gnu.org); Eric Christopher (echri...@gmail.com)
> Cc: Richard Sandiford; Rich Fuhler; Rozycki, Maciej; Myers, Joseph
> Subject: RE: [PATCHv4][MIPS] Implement O32 ABI extensions (GCC)
> 
> Sorry to follow myself up. I realised that the new configure options should be
> documented in install.texi. The only change from V3 is the doc/install.texi
> change.
> 
> The MIPS64 tests have completed without regression.
> 
> Regards,
> Matthew
> 
> gcc/
>   * common/config/mips/mips-common.c (mips_handle_option):
> Ensure
>   that -mfp32, -mfp64 disable -mfpxx and -mfpxx disables -mfp64.
>   * config.gcc (--with-fp-32): New option.
>   (--with-odd-spreg-32): Likewise.
>   * config.in (HAVE_AS_DOT_MODULE): New config define.
>   * config/mips/mips-protos.h
>   (mips_secondary_memory_needed): New prototype.
>   (mips_hard_regno_caller_save_mode): Likewise.
>   * config/mips/mips.c (mips_get_reg_raw_mode): New static
> prototype.
>   (mips_get_arg_info): Assert that V2SFmode is only handled specially
>   with TARGET_PAIRED_SINGLE_FLOAT.
>   (mips_return_mode_in_fpr_p): Likewise.
>   (mips16_call_stub_mode_suffix): Likewise.
>   (mips_get_reg_raw_mode): New static function.
>   (mips_return_fpr_pair): O32 return values span two registers.
>   (mips16_build_call_stub): Likewise.
>   (mips_function_value_regno_p): Support both FP return registers.
>   (mips_output_64bit_xfer): Use mthc1 whenever
> TARGET_HAS_MXHC1.  Add
>   specific cases for TARGET_FPXX to move via memory.
>   (mips_dwarf_register_span): For TARGET_FPXX pretend that modes
> larger
>   than UNITS_PER_FPREG 'span' one register.
>   (mips_dwarf_frame_reg_mode): New static function.
>   (mips_file_start): Switch to using .module instead of .gnu_attribute.
>   No longer support FP ABI 4 (-mips32r2 -mfp64), replace with FP ABI 6.
>   Add FP ABI 5 (-mfpxx) and FP ABI 7 (-mfp64 -mno-odd-spreg).
>   (mips_save_reg, mips_restore_reg): Always represent DFmode
> frame
>   slots with two CFI directives even for O32 FP64.
>   (mips_for_each_saved_gpr_and_fpr): Account for fixed_regs when
>   saving/restoring callee-saved registers.
>   (mips_hard_regno_mode_ok_p): Implement O32 FP64A extension.
>   (mips_secondary_memory_needed): New function.
>   (mips_option_override): ABI check for TARGET_FLOATXX.  Disable
>   odd-numbered single-precision registers when using
> TARGET_FLOATXX.
>   Implement -modd-spreg and defaults.
>   (mips_conditional_register_usage): Redefine O32 FP64 to match O32
> FP32
>   callee-saved behaviour.
>   (mips_hard_regno_caller_save_mode): Implement.
>   (TARGET_GET_RAW_RESULT_MODE): Define target hook.
>   (TARGET_GET_RAW_ARG_MODE): Define target hook.
>   (TARGET_DWARF_FRAME_REG_MODE): Define target hook.
>   * config/mips/mips.h (TARGET_FLOAT32): New macro.
>   (TARGET_O32_FP64A_ABI): Likewise.
>   (TARGET_CPU_CPP_BUILTINS): TARGET_FPXX is __mips_fpr==0.
> Add
>   _MIPS_SPFPSET builtin define.
>   (MIPS_FPXX_OPTION_SPEC): New macro.
>   (OPTION_DEFAULT_SPECS): Pass through --with-fp-32=* to -mfp and
>   --with-odd-spreg-32=* to -m[no-]odd-spreg.
>   (ISA_HAS_ODD_SPREG): New macro.
>   (ISA_HAS_MXHC1): True for anything other than -mfp32.
>   (ASM_SPEC): Pass through mfpxx, mfp64, -mno-odd-spreg and -
> modd-spreg.
>   (MIN_FPRS_PER_FMT): Redefine in terms of TARGET_ODD_SPREG.
>   (HARD_REGNO_CALLER_SAVE_MODE): Define.  Implement O32
> FPXX extension
>   (HARD_REGNO_CALL_PART_CLOBBERED): Likewise.
>   (SECONDARY_MEMORY_NEEDED): Likewise.
>   (FUNCTION_ARG_REGNO_P): Update for O32 FPXX and FP64
> extensions.
>   * config/mips/mips.md (define_attr enabled): Implement O32 FPXX
> and
>   FP64A ABI extensions.
>   (move_doubleword_fpr): Use ISA_HAS_MXHC1 instead of
>   TARGET_FLOAT64.
>   * config/mips/mips.opt (mfpxx): New target option.
>   (modd-spreg): Likewise.
>   * config/mips/mti-elf.h (DRIVER_SELF_SPECS): Infer FP ABI from
> arch.
>   * config/mips/mti-linux.h (DRIVER_SELF_SPECS): Likewise and
> remove
>   fp64 sysroot.
>   * config/mips/t-mti-elf: Remove fp64 multilib.
>   * config/mips/t-mti-linux: Likewise.
>   * configure.ac: Detect .module support.
>   * configure: Regenerate.
>   * doc/invoke.texi: Document -mfpxx, -modd-spreg, -mn

RE: [PATCHv4][MIPS] Implement O32 ABI extensions (GCC)

2014-11-12 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Wednesday, November 12, 2014 1:59 PM
> To: Moore, Catherine; 'gcc-patches@gcc.gnu.org' (gcc-
> patc...@gcc.gnu.org); Eric Christopher (echri...@gmail.com)
> Cc: Richard Sandiford; Rich Fuhler; Rozycki, Maciej; Myers, Joseph
> Subject: RE: [PATCHv4][MIPS] Implement O32 ABI extensions (GCC)
> 
> Moore, Catherine  writes:
> > The patch looks good.   Please fix up these couple of nits prior to
> > committing.
> 
> OK, thanks for the second read through. One further amendment below, I'll
> aim to commit later today.
> 

Yes, that's better.

> > Index: gcc/config/mips/mips.c
> >
> ==
> =
> > --- gcc/config/mips/mips.c  (revision 217363)
> > +++ gcc/config/mips/mips.c  (working copy)
> > @@ -18824,6 +19000,21 @@ mips_expand_vec_minmax (rtx target, rtx
> op0,
> > rtx o
> >emit_insn (gen_rtx_SET (VOIDmode, target, x));  }
> >
> > +/* Implement HARD_REGNO_CALLER_SAVE_MODE.  */
> > +
> > +machine_mode
> > +mips_hard_regno_caller_save_mode (unsigned int regno,
> > + unsigned int nregs,
> > + machine_mode mode) {
> > +  /* For performance, to avoid saving/restoring upper parts of a
> > register,
> > + we return MODE as save mode when MODE is not VOIDmode.  */
> >
> > s/performance, to/performance, /
> >
> 
> The second part of this sentence will need to change too I think:
> 
> For performance, avoid saving/restoring upper parts of a register by
> returning MODE as save mode when the mode is known.
> 
> Thanks,
> Matthew


RE: [PATCH, testsuite] MIPS: Cleanup the forcing of assembly output in error tests.

2016-12-22 Thread Moore, Catherine


> -Original Message-
> From: Toma Tabacu [mailto:toma.tab...@imgtec.com]
> Sent: Wednesday, December 14, 2016 9:56 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Matthew Fortune ; Moore,
> Catherine 
> Subject: [PATCH, testsuite] MIPS: Cleanup the forcing of assembly
> output in error tests.
> 
> Hi,
> 
> Some error tests were forcing assembly output incorrectly, and none
> of them had
> an explanation for why they were using -ffat-lto-objects.
> 
> This patch removes dg-skip-if's for -fno-fat-lto-objects and
> adds -ffat-lto-objects to the test options instead.
> It also adds an explanation for the purpose of -ffat-lto-objects in each
> test.
> 
> Tested with mips-mti-elf.
> 
> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.target/mips/oddspreg-2.c (dg-options): Remove dg-skip-if
> for
>   -fno-fat-lto-objects and add the -ffat-lto-objects option, along
> with
>   an explanation for its purpose.
>   * gcc.target/mips/oddspreg-3.c (dg-options): Likewise.
>   * gcc.target/mips/oddspreg-6.c (dg-options): Likewise.
>   * gcc.target/mips/no-dsp-1.c: Add an explanation for the
> purpose of
>   -ffat-lto-objects.
>   * gcc.target/mips/pr54240.c: Likewise.
>   * gcc.target/mips/r10k-cache-barrier-14.c: Likewise.
>   * gcc.target/mips/soft-float-1.c: Likewise.
> 

This is OK.


RE: [PATCH, MIPS] Target flag and build option to disable indexed memory OPs.

2017-01-16 Thread Moore, Catherine


> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Matthew Fortune
> Sent: Monday, January 16, 2017 11:25 AM
> To: Doug Gilmore ; gcc-
> patc...@gcc.gnu.org
> Cc: Moore, Catherine 
> Subject: RE: [PATCH, MIPS] Target flag and build option to disable
> indexed memory OPs.
> 
> Doug Gilmore 
> > I recently bisected PR78176 to problems introduced with r21650.
> >
> > Given the short time until the release, we would like to provide a
> > target flag and build option to avoid the bug until we are able to
> > resolve the problem with the commit.  Note that as Matthew Fortune
> has
> > mentioned in the PR:
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78176#c5
> >
> > the problem could also be addressed by updates to the Linux kernel
> since
> > the problem is only exposed by running MIPS 32-bit binaries on 64-
> bit
> > kernels.
> >
> > Bootstrapped on X86_64, regression tested on X86_64 and MIPS.
> >
> > OK to commit?
> 
> Given this is a generic reference to indexed load/store and the issue
> could
> affect any indexed operation then I think it needs to include all of the
> following as well:
> 
> /* ISA has lwxs instruction (load w/scaled index address.  */
> #define ISA_HAS_LWXS((TARGET_SMARTMIPS ||
> TARGET_MICROMIPS) \
>  && !TARGET_MIPS16)
> 
> /* ISA has lbx, lbux, lhx, lhx, lhux, lwx, lwux, or ldx instruction. */
> #define ISA_HAS_LBX (TARGET_OCTEON2)
> #define ISA_HAS_LBUX(ISA_HAS_DSP || TARGET_OCTEON2)
> #define ISA_HAS_LHX (ISA_HAS_DSP || TARGET_OCTEON2)
> #define ISA_HAS_LHUX(TARGET_OCTEON2)
> #define ISA_HAS_LWX (ISA_HAS_DSP || TARGET_OCTEON2)
> #define ISA_HAS_LWUX(TARGET_OCTEON2 && TARGET_64BIT)
> #define ISA_HAS_LDX ((ISA_HAS_DSP || TARGET_OCTEON2) \
>  && TARGET_64BIT)
> 
> The DSP LBUX/LHX/LWX/LDX intrinsics will also need a new AVAIL
> predicate
> to disable them. The snag is that some DSP code will fail to compile if it
> uses the DSP load intrinsics directly.
> 
> I see no way of avoiding that. Therefore, distributions that use
> --without-indexed-load-store will have to cope with some potential
> DSP
> fallout if they enable DSP at all.
> 
> @Catherine: I'd like your input here if possible as I advocated this
> approach, comments on option names welcome too.  I quite like the
> verbose
> name.

Okay, based on my reading of the comments in the bug report, you are proposing 
this option as a workaround to a kernel deficiency.  I don't see any agreement 
that this is actually a compiler bug.
Do we really need to include the DSP instrinsics as well?   Do you think that 
many distributions actually enable DSP?  

The option name itself is acceptable to me.  I'd like to see documentation that 
explains when this problem is exposed.  I'd like to limit the fix to LWXS and 
I'd like to see the testcase from the bug report added to the testsuite.
I also agree that the preprocessor macro is a good idea (even if we decide to 
forgo the DSP portion of the patch).

Catherine

> 
> @Doug: Have you tried running the testsuite with the configure option
> --without-indexed-load-store? There may be tests that need adjusting
> where they
> test indexed load/store. We probably need a pre-processor macro
> to detect if the option is enabled/disabled so that DSP code can be
> predicated
> on indexed load being available.
> 
> Thanks,
> Matthew


RE: [PATCH, MIPS] Target flag and build option to disable indexed memory OPs.

2017-01-17 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Tuesday, January 17, 2017 4:35 AM
> To: Moore, Catherine ; Doug
> Gilmore ; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH, MIPS] Target flag and build option to disable
> indexed memory OPs.
> 
> Moore, Catherine  writes:
> > > -Original Message-
> > > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> > > ow...@gcc.gnu.org] On Behalf Of Matthew Fortune
> > > Sent: Monday, January 16, 2017 11:25 AM
> > > To: Doug Gilmore ; gcc-
> > > patc...@gcc.gnu.org
> > > Cc: Moore, Catherine 
> > > Subject: RE: [PATCH, MIPS] Target flag and build option to disable
> > > indexed memory OPs.
> > >
> > > Doug Gilmore 
> > > > I recently bisected PR78176 to problems introduced with r21650.
> > > >
> > > > Given the short time until the release, we would like to provide a
> > > > target flag and build option to avoid the bug until we are able to
> > > > resolve the problem with the commit.  Note that as Matthew
> Fortune
> > > has
> > > > mentioned in the PR:
> > > >
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78176#c5
> > > >
> > > > the problem could also be addressed by updates to the Linux
> kernel
> > > since
> > > > the problem is only exposed by running MIPS 32-bit binaries on
> 64-
> > > bit
> > > > kernels.
> > > >
> > > > Bootstrapped on X86_64, regression tested on X86_64 and MIPS.
> > > >
> > > > OK to commit?
> > >
> > > Given this is a generic reference to indexed load/store and the
> issue
> > > could
> > > affect any indexed operation then I think it needs to include all of
> the
> > > following as well:
> > >
> > > /* ISA has lwxs instruction (load w/scaled index address.  */
> > > #define ISA_HAS_LWXS((TARGET_SMARTMIPS ||
> > > TARGET_MICROMIPS) \
> > >  && !TARGET_MIPS16)
> > >
> > > /* ISA has lbx, lbux, lhx, lhx, lhux, lwx, lwux, or ldx instruction. */
> > > #define ISA_HAS_LBX (TARGET_OCTEON2)
> > > #define ISA_HAS_LBUX(ISA_HAS_DSP || TARGET_OCTEON2)
> > > #define ISA_HAS_LHX (ISA_HAS_DSP || TARGET_OCTEON2)
> > > #define ISA_HAS_LHUX(TARGET_OCTEON2)
> > > #define ISA_HAS_LWX (ISA_HAS_DSP || TARGET_OCTEON2)
> > > #define ISA_HAS_LWUX(TARGET_OCTEON2 &&
> TARGET_64BIT)
> > > #define ISA_HAS_LDX ((ISA_HAS_DSP || TARGET_OCTEON2)
> \
> > >  && TARGET_64BIT)
> > >
> > > The DSP LBUX/LHX/LWX/LDX intrinsics will also need a new AVAIL
> > > predicate
> > > to disable them. The snag is that some DSP code will fail to compile
> if it
> > > uses the DSP load intrinsics directly.
> > >
> > > I see no way of avoiding that. Therefore, distributions that use
> > > --without-indexed-load-store will have to cope with some potential
> > > DSP
> > > fallout if they enable DSP at all.
> > >
> > > @Catherine: I'd like your input here if possible as I advocated this
> > > approach, comments on option names welcome too.  I quite like
> the
> > > verbose
> > > name.
> >
> > Okay, based on my reading of the comments in the bug report, you
> are proposing this option
> > as a workaround to a kernel deficiency.  I don't see any agreement
> that this is actually a
> > compiler bug.
> > Do we really need to include the DSP instrinsics as well?   Do you
> think that many
> > distributions actually enable DSP?
> >
> > The option name itself is acceptable to me.  I'd like to see
> documentation that explains
> > when this problem is exposed.  I'd like to limit the fix to LWXS and I'd
> like to see the
> > testcase from the bug report added to the testsuite.
> > I also agree that the preprocessor macro is a good idea (even if we
> decide to forgo the
> > DSP portion of the patch).
> 
> Thanks for the comments.
> 
> Having thought further I agree we can safely ignore DSP indexed load
> and micromips LWXS on
> the basis that DSP code will not run on a MIPS64 processor anyway (at
> least none that I
> know of) so the issue cannot occur and similarly for microMIPS, there
> are no 64-bit cores.
> 
> Restricting to just LWXC1/SWXC1/LDXC1/SDXC1 is therefore fine but
> we should reflect
> that in option names then.
> 
> --with-lxc1-sxc1 --without-lxc1-sxc1
> -mlxc1-sxc1
> 
> These names reflect the internal macro that controls availability of
> these instructions.
> 
> Macro name: __mips_no_lxc1_sxc1
> Defined when !ISA_HAS_LXC1_SXC1 so would be present even when
> targeting a core that
> doesn't have the instructions anyway.
> 
> Any refinements to this Catherine?
> 
No.  This plan looks good.


RE: [patch mips/gcc] add build-time and runtime options to disable or set madd.fmt type

2017-01-19 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Thursday, January 19, 2017 5:30 PM
> To: Moore, Catherine 
> Cc: 'Aurelien Jarno' ; 'Richard Sandiford'
> ; Loosemore, Sandra
> ; Yunqiang Su
> ; 'gcc-patches' 
> Subject: RE: [patch mips/gcc] add build-time and runtime options to
> disable or set madd.fmt type
> 
> Matthew Fortune  writes:
> > I've rewritten/simplified this patch as it provides far too much
> control
> > to end users who will undoubtedly shoot themselves in the foot so to
> > speak. The option I intend to support is simply --with-madd4 --
> without-madd4
> > and -mmadd4 -mno-madd4. This is a simple enable/disable on top of
> > architecture checks to use/not use the madd4 family of instructions.
> >
> > We have to keep each of these unusual features simple so that we
> can somehow
> > reason about them in the future.
> >
> 
> Here is the tested patch.  Configure time default set/not set tested and
> testsuite
> fixes in place to deal with the fallout from running with the madd4
> instructions
> disabled.  Tests done with an o32 config on mips64el-linux-gnu.  If
> there is any
> other fallout from other test configurations I'll catch those as I try to
> get the
> rest of the testsuite issues resolved before release.
> 
> Catherine, any issues to raise on this new option?

I committed this patch after fixing a couple of typos in the documentation and 
ChangeLog entry.
No other objections.
Catherine

> 
> Thanks,
> Matthew
> 
> gcc/
> 
>   * config.gcc (supported_defaults): Add madd4.
>   (with_madd4): Add validation.
>   (all_defaults): Add madd4.
>   * config/mips/mips.opt (mmadd4): New option.
>   * gcc/config/mips/mips.h (OPTION_DEFAULT_SPECS): Add a
> default for
>   mmadd4.
>   (TARGET_CPU_CPP_BUILTINS): Add builtin_define for
>   __mips_no_madd4.
>   (ISA_HAS_UNFUSED_MADD4): Gate with mips_madd4.
>   (ISA_HAS_FUSED_MADD4): Likewise.
>   * gcc/doc/invoke.texi (-mmadd4): Document the new option.
>   * doc/install.texi (--with-madd4): Document the new option.
> 
> gcc/testsuite/
> 
>   * gcc.target/mips/madd4-1.c: New file.
>   * gcc.target/mips/madd4-2.c: Likewise.
>   * gcc.target/mips/mips.exp (mips_option_groups): Add ghost
> option
>   HAS_MADD4.
>   (mips_option_groups): Add -m[no-]madd4.
>   (mips-dg-init): Detect default -mno-madd4.
>   (mips-dg-options): Handle HAS_MADD4 arch
> upgrade/downgrade.
>   * gcc.target/mips/mips-ps-type.c: Add -mmadd4 test option.
>   * gcc.target/mips/mips-ps-type-2.c: Likewise.
>   * gcc.target/mips/nmadd-1.c: Likewise.
>   * gcc.target/mips/nmadd-2.c: Likewise.
>   * gcc.target/mips/nmadd-3.c: Likewise.
> ---
>  gcc/ChangeLog  | 16 
>  gcc/config.gcc | 19 +--
>  gcc/config/mips/mips.h | 12 +---
>  gcc/config/mips/mips.opt   |  4 
>  gcc/doc/install.texi   | 14 ++
>  gcc/doc/invoke.texi|  8 +++-
>  gcc/testsuite/ChangeLog| 15 +++
>  gcc/testsuite/gcc.target/mips/madd4-1.c| 14 ++
>  gcc/testsuite/gcc.target/mips/madd4-2.c| 14 ++
>  gcc/testsuite/gcc.target/mips/mips-ps-type-2.c |  2 +-
>  gcc/testsuite/gcc.target/mips/mips-ps-type.c   |  2 +-
>  gcc/testsuite/gcc.target/mips/mips.exp | 12 +++-
>  gcc/testsuite/gcc.target/mips/nmadd-1.c|  2 +-
>  gcc/testsuite/gcc.target/mips/nmadd-2.c|  2 +-
>  gcc/testsuite/gcc.target/mips/nmadd-3.c|  2 +-
>  15 files changed, 126 insertions(+), 12 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/mips/madd4-1.c
>  create mode 100644 gcc/testsuite/gcc.target/mips/madd4-2.c
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index e53f9e1..7496071 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,19 @@
> +2017-01-19  Matthew Fortune  
> + Yunqiang Su  
> +
> + * config.gcc (supported_defaults): Add madd4.
> + (with_madd4): Add validation.
> + (all_defaults): Add madd4.
> + * config/mips/mips.opt (mmadd4): New option.
> + * gcc/config/mips/mips.h (OPTION_DEFAULT_SPECS): Add a
> default for
> + mmadd4.
> + (TARGET_CPU_CPP_BUILTINS): Add builtin_define for
> + __mips_no_madd4.
> + (ISA_HAS_UNFUSED_MADD4): Gate with mips_madd4.
> + (ISA_HAS_FUSED_MADD4): Likewise.
&

RE: [PATCH,testsuite] Skip gcc.dg/pic-2.c and gcc.dg/pie-2.c for MIPS.

2017-03-20 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Wednesday, March 15, 2017 11:37 AM
> To: Moore, Catherine 
> Cc: Toma Tabacu ; gcc-
> patc...@gcc.gnu.org
> Subject: RE: [PATCH,testsuite] Skip gcc.dg/pic-2.c and gcc.dg/pie-2.c for
> MIPS.
> 
> Toma Tabacu  writes:
> > The gcc.dg/pic-2.c and gcc.dg/pie-2.c tests are failing for MIPS targets
> > because __PIC__ is always set to 1 for MIPS.
> >
> > This patch makes the testsuite skip those two tests for all MIPS
> > targets.
> >
> > Tested with mips-mti-elf and mips-mti-linux-gnu.
> >
> > Should I have fixed this in target-supports.exp instead ?
> > I was worried that doing so would complicate the fpic and pie effective
> > target checks too much.
> 
> I think the skip is OK here. I'd like to get Catherine's opinion on
> this though too. I don't think we should change the definition of __PIC__
> for -fPIC on MIPS as multi-got solves 'most' issues. If we start trying to
> figure out what __PIC__ should mean on MIPS then we will get into a big
> mess with -mxgot as that is arguably __PIC__==2 but I expect there will
> be  several differing opinions.
> 
I think the skip is the right way to go as well.   The patch is OK with me.
Catherine




RE: [PATCH] Fix MIPS-specific ICE in gcc.dg/pr77834.c (PR rtl-optimization/79150).

2017-03-20 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Wednesday, March 15, 2017 11:30 AM
> To: Moore, Catherine 
> Cc: Segher Boessenkool (seg...@kernel.crashing.org)
> ; Toma Tabacu
> ; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Fix MIPS-specific ICE in gcc.dg/pr77834.c (PR rtl-
> optimization/79150).
> 
> Hi Catherine,
> 
> What is your opinion on this patch? I am OK with the temporary
> workaround on the basis that the additional nop is successfully
> eliminated so there is no code size increase. Also, I am happy
> enough that the CFG is fixed very soon after the block move is
> expanded so the 'bad' basic block is fixed almost immediately
> anyway making the offending check potentially unnecessary in
> the first place.
> 
I'm okay with the workaround for stage 4, but would like to see the pr remain 
open until a proper fix is installed on trunk. 
Toma, would you be able to pursue the original patch that you attached to the 
bug report?

Catherine
> 
> > -Original Message-
> > From: Toma Tabacu
> > Sent: 07 March 2017 11:44
> > To: gcc-patches@gcc.gnu.org
> > Cc: Matthew Fortune; Segher Boessenkool
> (seg...@kernel.crashing.org)
> > Subject: [PATCH] Fix MIPS-specific ICE in gcc.dg/pr77834.c (PR rtl-
> > optimization/79150).
> >
> > Hi,
> >
> > This ICE is caused by "gcc_assert (!JUMP_P (last))" in
> > commit_one_edge_insertion (gcc/cfgrtl.c):
> >
> >   if (returnjump_p (last))
> > {
> >   /* ??? Remove all outgoing edges from BB and add one for EXIT.
> >  This is not currently a problem because this only happens
> >  for the (single) epilogue, which already has a fallthru edge
> >  to EXIT.  */
> >
> >   e = single_succ_edge (bb);
> >   gcc_assert (e->dest == EXIT_BLOCK_PTR_FOR_FN (cfun)
> >   && single_succ_p (bb) && (e->flags &
> EDGE_FALLTHRU));
> >
> >   e->flags &= ~EDGE_FALLTHRU;
> >   emit_barrier_after (last);
> >
> >   if (before)
> > delete_insn (before);
> > }
> >   else
> > gcc_assert (!JUMP_P (last));
> >
> > The assert is triggered because we're removing an edge which ends on a
> > conditional jump, which is part of a loop.
> >
> > The reason for the existence of this loop is the size of the vector used
> > in gcc.dg/pr77834.c.
> > When compiling code which uses vectors for MIPS, a looping memcpy is
> > generated if the vectors are big enough (>32 bytes for MIPS32 or >64
> > bytes for MIPS64).
> > This takes place in mips_expand_block_move (gcc/config/mips.c).
> >
> > In our case, a looping memcpy gets generated for a partition copy
> which
> > is inserted on an edge (in insert_partition_copy_on_edge, gcc/tree-
> > outof-ssa.c).
> > This happens during PHI node elimination, which is done by
> eliminate_phi
> > (gcc/tree-outof-ssa.c), as a part of expand_phi_nodes, which is called
> > by the expand pass (gcc/cfgexpand.c).
> >
> > My original fix was to update the CFG by calling
> > find_many_sub_basic_blocks with an all-one block bitmap (which also
> > happens in cfgexpand.c, after edge
> > removal) whenever an edge contains an insn which satisfies
> > control_flow_insn_p.
> > However, Segher Boessenkool said that this would be too risky for stage
> > 4 and suggested inserting a NOP after the conditional jump in order to
> > fool the assert. This assumes that it is safe to delay the CFG update
> > for this particular case.
> >
> > This patch changes mips_block_move_loop to emit a NOP after the
> > conditional jump, if the jump is the last insn of the loop. This
> > prevents "gcc_assert (!JUMP_P (last))" from triggering.
> >
> > The NOP will never make it into the final assembly code because it is
> > removed during the cse1 pass through DCE for -O1, -O2, and -O3, and
> it's
> > not even emitted for -O0 and -Os because looping memcpy's are not
> > generated for those optimization levels, as can be seen in
> > mips_expand_block_move from mips.c:
> >
> >   if (INTVAL (length) <= MIPS_MAX_MOVE_BYTES_STRAIGHT)
> > {
> >   mips_block_move_straight (dest, src, INTVAL (length));
> >   return true;
> > }
> >   else if (optimize)
> > {
> >   mips_block_move_loop (dest, src, INTVAL (length),
> >
>   MIPS_MAX_MOVE_BYTES_PER_LOOP_ITER);
> >   return true;
> > }
> >
> > Tested with m

RE: [patch, MIPS] Update -mvirt option description

2017-04-06 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Friday, March 31, 2017 4:00 PM
> To: Moore, Catherine 
> Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)  patc...@gcc.gnu.org>; roland.il...@gmx.de
> Subject: [patch, MIPS] Update -mvirt option description
> 
> Hi Catherine,
> 
> I'm following up on PR target/80057 to update the description of -
> mvirt.
> 
> I agree with Roland that the description is inconsistent and should not
> state 'application specific' as none of the other ASE options include
> it. Instead I suggest we put the shortened form of (VZ) in like (XPA)
> is shown for -mxpa. The short form of virtualization ASE is usually VZ
> not VIRT which has always irritated me about this option name but
> that
> is history.
> 
> What do you think?

This looks good to me.
> 
> gcc/
>   PR target/80057
>   * config/mips/mips.opt (-mvirt): Update description.
> 




RE: [PATCH][MIPS] MSA machine description fixes

2017-02-21 Thread Moore, Catherine
> The patch fixes some bugs as mentioned below.
> 
> 1. mips_gen_const_int_vector(): Change type for argument VAL from
> int to HOST_WIDE_INT to allow const vector of type doubleword. It in
> turn enables generation of BCLRI.d instead of AND.d for immediate
> const vector operand with only one bit clear.
> 
> 2. MSA dot product family instruction for .d format: Fix wrong MODE
> for vec_select in the second mult operand. It enables some
> optimizations like CSE fwprop etc.
> 
> 3. signed min/max immediate: Fix print operand code so as to print
> signed immediate instead of an unsigned one.
> 
> 4. MSA max/min absolute instruction family: Introduce mode iterator
> in if_then_else construct. It enables some optimizations like CSE
> fwprop etc.
> 
> Tests for all of them are also included in the patch.
> 
> Ok for trunk?
> 
> Changelog:
> 
> 2017-02-07  Prachi Godbole  
> 
> gcc/
>   * config/mips/mips-msa.md (msa_dotp__d,
> msa_dpadd__d,
>   msa_dpsub__d): Fix MODE for vec_select.
>   (msa_fmax_a_, msa_fmin_a_,
> msa_max_a_,
>   msa_min_a_): Introduce mode interator for
> if_then_else.
>   (smin3, smax3): Change operand print code
> from 'B' to 'E'.
>   * config/mips/mips.c (mips_gen_const_int_vector): Change
> type of last
>   argument.
>   * config/mips/mips-protos.h (mips_gen_const_int_vector):
> Likewise.
> 
Hi Prachi,

This part of the patch is fine, although I would like you to split it into 
three patches giving a separate commit for each bug fix.

> gcc/testsuite/
>   * gcc.target/mips/msa-1.c: New tests.

The testsuite patch should be split be split as well.  I have a couple of 
comments on the test itself.
> 
> 
> Index: gcc/testsuite/gcc.target/mips/msa-1.c
> ==
> =
> --- gcc/testsuite/gcc.target/mips/msa-1.c   (revision 0)
> +++ gcc/testsuite/gcc.target/mips/msa-1.c   (revision 0)
> @@ -0,0 +1,69 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mno-mips16 -mfp64 -mhard-float -mmsa" } */
> +
> +typedef int v4i32 __attribute__ ((vector_size(16)));
> +typedef long long v2i64 __attribute__ ((vector_size(16)));
> +typedef float v4f32 __attribute__ ((vector_size(16)));
> +
> +/* Test MSA AND.d optimization: generate BCLRI.d instead, for
> immediate const
> +   vector operand with only one bit clear.  */
> +
> +void
> +and_d_msa (v2i64 *vx, v2i64 *vy)
> +{
> +  v2i64 and_vec = {0x7FFF, 0x7FFF};
> +  *vy = (*vx) & and_vec;
> +}

I know that compiler crashed for this test, but let's test for the generation 
of the BCLRI as well.

> +
> +/* Test MSA dot product family for CSE optimization.  */
> +
> +static v4i32 g = {0, 92, 93, 94};
> +static v4i32 h = {12, 24, 36, 48};
> +static v2i64 l = {84, 98};
> +
> +void
> +dotp_d_msa (v2i64 *c)
> +{
> +  l = __builtin_msa_dotp_s_d (g, h);
> +}
> +
> +void
> +dpadd_d_msa (v2i64 *c)
> +{
> +  *c = __builtin_msa_dpadd_s_d (l, g, h);
> +}
> +
> +void
> +dpsub_d_msa (v2i64 *c)
> +{
> +  *c = __builtin_msa_dpsub_s_d (l, g, h);
> +}

Likewise.  Can you test the assembly or one of the dumps for the optimization 
that you're looking for?

> +
> +/* Test MSA signed min/max immediate for correct assembly output.
> */
> +
> +void
> +min_s_msa (v4i32 *vx, v4i32 *vy)
> +{
> +  *vy = __builtin_msa_mini_s_w (*vx, -15);
> +}
> +/* { dg-final { scan-assembler "-15" } }  */
> +
> +void
> +max_s_msa (v4i32 *vx, v4i32 *vy)
> +{
> +  *vy = __builtin_msa_maxi_s_w (*vx, -15);
> +}
> +/* { dg-final { scan-assembler "-15" } }  */
> +
> +/* Test MSA min_a/max_a instructions for forward propagation
> optimization.  */
> +
> +#define FUNC(NAME, TYPE, RETTYPE) RETTYPE NAME##_a_msa (TYPE
> *vx, TYPE *vy) \
> +{ \
> +  TYPE dest = __builtin_msa_##NAME##_a_w (*vx, *vy); \
> +  return dest[0]; \
> +}
> +
> +FUNC(fmin, v4f32, float)
> +FUNC(fmax, v4f32, float)
> +FUNC(min, v4i32, int)
> +FUNC(max, v4i32, int)

This is OK.

Please repost the patches with the modifications.
Thanks,
Catherine


RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA

2017-02-21 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Tuesday, February 21, 2017 5:35 AM
> To: Sameera Deshpande ; Moore,
> Catherine 
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH, MIPS] Calling convention differs depending on the
> presence of MSA
> 
> 
> As this is an ABI fix, just wait a day or so in case Catherine has
> any comment otherwise OK to commit.
> 
I have not  further comments on this patch.  Please commit.


RE: [PATCH,MIPS] Document -mload-store-pairs

2017-02-24 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Friday, February 24, 2017 8:58 AM
> To: Moore, Catherine 
> Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)  patc...@gcc.gnu.org>
> Subject: [PATCH,MIPS] Document -mload-store-pairs
> 
> Hi Catherine,
> 
> Can you review the description for -mload-store-pairs please?
> 
> Thanks,
> Matthew
> 
> gcc/
>   PR target/79473
>   * doc/invoke.texi: Document -mload-store-pairs.
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 6e5fa56..f1fc449 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -879,6 +879,7 @@ Objective-C and Objective-C++ Dialects}.
>  -mexplicit-relocs  -mno-explicit-relocs @gol
>  -mcheck-zero-division  -mno-check-zero-division @gol
>  -mdivide-traps  -mdivide-breaks @gol
> +-mload-store-pairs  -mno-load-store-pairs @gol
>  -mmemcpy  -mno-memcpy  -mlong-calls  -mno-long-calls @gol
>  -mmad  -mno-mad  -mimadd  -mno-imadd  -mfused-madd  -mno-fused-
> madd  -nocpp @gol
>  -mfix-24k  -mno-fix-24k @gol
> @@ -19495,6 +19496,16 @@ overridden at configure time using
> @option{--with-divide=breaks}.
>  Divide-by-zero checks can be completely disabled using
>  @option{-mno-check-zero-division}.
> 
> +@item -mload-store-pairs
> +@itemx -mno-load-store-pairs
> +@opindex mload-store-pairs
> +@opindex mno-load-store-pairs
> +Enable (disable) an optimization that keeps consecutive load or store
> +instructions sequential to allow MIPS processors that perform load
> +and store bonding to optimize the access.  This option is enabled by
> +default but only takes effect when the selected architecture is known
> +to support bonding.
> +
>  @item -mmemcpy
>  @itemx -mno-memcpy
>  @opindex mmemcpy
> --
> 2.2.1

Hi Matthew -- How about this instead?

+@item -mload-store-pairs
+@itemx -mno-load-store-pairs
+@opindex mload-store-pairs
+@opindex mno-load-store-pairs
+Enable (disable) an optimization that pairs consecutive load or store
+instructions to enable load/store bonding.  This option is enabled by
+default but only takes effect when the selected architecture is known
+to support bonding.
+


RE: [PATCH,MIPS] Handle paired single test changes

2017-02-24 Thread Moore, Catherine


> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Matthew Fortune
> Sent: Thursday, February 23, 2017 5:21 PM
> To: Moore, Catherine 
> Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)  patc...@gcc.gnu.org>
> Subject: [PATCH,MIPS] Handle paired single test changes
> 
> Hi Catherine,
> 
> I missed a couple of testsuite changes that are needed to deal with the
> fallout of fixing the ABI issues for floating point vectors.  I had them
> in my tree but forgot to post.  The ABI change for V2SF i.e. paired
> single is a bug fix as the behaviour was unintended and violates the goal
> of having FP64 a compatible ABI extension for o32.  The probability of
> having code dependent on this corner case of the calling convention in
> the wild is exceptionally low so I see no significant risk still.
> 
> The tests for paired single just need a little encouragement to still
> produce the necessary instructions now that paired single is not returned
> in registers.
> 
> Does it look OK to you?
> 
> Thanks,
> Matthew
> 
> gcc/testsuite/
> 
>   * gcc.target/mips/mips-ps-type-2.c (move): Force generation
>   of mov.ps.
>   * gcc.target/mips/mips-ps-type.c (move): Likewise.
>   (cond_move1): Simplify condition to force generation of
>   mov[nz].ps.
>   (cond_move2): Likewise.

Hi Matthew -- Looks good to me.
Catherine



RE: [PATCH,testsuite] Skip gcc.dg/lto/pr60449_0.c for mips*-*-elf* targets.

2017-02-28 Thread Moore, Catherine


> -Original Message-
> From: Toma Tabacu [mailto:toma.tab...@imgtec.com]
> Sent: Tuesday, February 28, 2017 9:32 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Matthew Fortune ; Moore,
> Catherine 
> Subject: [PATCH,testsuite] Skip gcc.dg/lto/pr60449_0.c for mips*-*-elf*
> targets.
> 
> Hi,
> 
> The gcc.dg/lto/pr60449_0.c is failing for mips*-*-elf* targets because
> they do
> not support gettimeofday, which is used in this test case.
> 
> This patch changes the test so that it is skipped for mips*-*-elf* targets.
> 

Hi Toma, 
There are some MIPS ELF targets that do support gettimeofday.   Perhaps you 
could handle this with a dg_require_effective_target entry for gettimeofday.
Thanks,
Catherine

> 
> gcc/testsuite/
> 
>   * gcc.dg/lto/pr60449_0.c (dg-skip-if): Skip for mips*-*-elf*.
> 
> diff --git a/gcc/testsuite/gcc.dg/lto/pr60449_0.c
> b/gcc/testsuite/gcc.dg/lto/pr60449_0.c
> index 5b878a6..6f3eccb 100644
> --- a/gcc/testsuite/gcc.dg/lto/pr60449_0.c
> +++ b/gcc/testsuite/gcc.dg/lto/pr60449_0.c
> @@ -1,5 +1,5 @@
>  /* { dg-lto-do link } */
> -/* { dg-skip-if "Needs gettimeofday" { "avr-*-*" } } */
> +/* { dg-skip-if "Needs gettimeofday" { "avr-*-*" mips*-*-elf* } } */
> 
>  extern int printf (const char *__restrict __format, ...);
>  typedef long int __time_t;



RE: [PATCH,testsuite] Skip gcc.dg/lto/pr60449_0.c for mips*-*-elf* targets.

2017-03-02 Thread Moore, Catherine


> -Original Message-
> From: Toma Tabacu [mailto:toma.tab...@imgtec.com]
> Sent: Thursday, March 2, 2017 11:47 AM
> To: Rainer Orth 
> Cc: Moore, Catherine ; gcc-
> patc...@gcc.gnu.org; Matthew Fortune
> 
> Subject: RE: [PATCH,testsuite] Skip gcc.dg/lto/pr60449_0.c for mips*-*-
> elf* targets.
> 
> Hi,
> 
> > From: Rainer Orth
> > >
> > > gcc/testsuite/
> > >
> > >   * gcc.dg/lto/pr60449_0.c: Add dg-require-effective-target for
> > >   gettimeofday. Remove dg-skip-if for AVR.
> >
> > Two spaces after period.
> >
> 
> Fixed.
> 
> > >   * lib/target-supports.exp (check_effective_target_gettimeofday):
> New.
> >
> > Better say "New proc." or something like this.
> >
> 
> Fixed.
> 
> >
> > Thanks for your patience.
> >
> > Rainer
> 
> No problem.
> Thanks for the review.
> 
> Below is the version with the updated ChangeLog entries.
> 
> Catherine, Matthew, what do you think ?
> 

This patch fixes my original objection.  Thanks.




RE: install.texi and mips-*-* (was: Target maintainers: doc/install.texi love and care)

2017-03-12 Thread Moore, Catherine


> -Original Message-
> From: Gerald Pfeifer [mailto:ger...@pfeifer.com]
> Sent: Sunday, March 12, 2017 7:38 AM
> To: gcc-patches@gcc.gnu.org; Moore, Catherine
> ; Matthew Fortune
> 
> Subject: install.texi and mips-*-* (was: Target maintainers: doc/install.texi
> love and care)
> 
> On Sun, 12 Mar 2017, Gerald Pfeifer wrote:
> > References to dependencies on really, really old versions of
> > binutils (talking 10+ years here) which I think we can remove.
> > Let me follow-up with some of you with concrete suggestions
> > around that.
> 
> The mips-*-* currently has this:
> 
>   The assembler from GNU binutils 2.17 and earlier has a bug in the way
>   it sorts relocations for REL targets (o32, o64, EABI).  This can cause
>   bad code to be generated for simple C++ programs.  Also the linker
>   from GNU binutils versions prior to 2.17 has a bug which causes the
>   runtime linker stubs in very large programs to
>   be incorrectly generated.  GNU Binutils 2.18 and later (and snapshots
>   made after Nov. 9, 2006) should be free from both of these problems.
> 
> (Even that goes back more than 10 years.)
> 
> Okay to yank it?

Yes, thank you.  I will review the rest of the MIPS doc in install.texi this 
week.
Catherine



[Patch][MIPS} [Committed] Fix gcc.dg/pch.exp failures for microMIPS multilibs

2016-10-14 Thread Moore, Catherine
I've committed this patch to fix failures in  g++.dg/pch.exp and gcc.dg/pch.exp 
tests when compiling with -mmicromips.  Invalid data was being read from the 
precompiled-header causing a segfault while trying to switch compression 
context.
This patch fixes that segfault by initializing micromips_globals to zero prior 
to writing the PCH file.  

Thanks,
Catherine

2016-10-13  Catherine Moore  

* gcc/config/mips/mips.c (mips_prepare_pch_save): Initialize
micromips_globals to zero.

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 43174b4..ebec68e 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -20856,28 +20856,30 @@ mips_shift_truncation_mask (machine_mode mode)
 static void
 mips_prepare_pch_save (void)
 {
-  /* We are called in a context where the current MIPS16 vs. non-MIPS16
- setting should be irrelevant.  The question then is: which setting
- makes most sense at load time?
+  /* We are called in a context where the current compression vs.
+ non-compression setting should be irrelevant.  The question then is:
+ which setting makes most sense at load time?

- The PCH is loaded before the first token is read.  We should never
- have switched into MIPS16 mode by that point, and thus should not
- have populated mips16_globals.  Nor can we load the entire contents
- of mips16_globals from the PCH file, because mips16_globals contains
- a combination of GGC and non-GGC data.
+ The PCH is loaded before the first token is read.  We should never have
+ switched into a compression mode by that point, and thus should not have
+ populated mips16_globals or micromips_globals.  Nor can we load the
+ entire contents of mips16_globals or micromips_globals from the PCH file,
+ because they contain a combination of GGC and non-GGC data.

  There is therefore no point in trying save the GGC part of
- mips16_globals to the PCH file, or to preserve MIPS16ness across
- the PCH save and load.  The loading compiler would not have access
- to the non-GGC parts of mips16_globals (either from the PCH file,
- or from a copy that the loading compiler generated itself) and would
- have to call target_reinit anyway.
-
- It therefore seems best to switch back to non-MIPS16 mode at
- save time, and to ensure that mips16_globals remains null after
- a PCH load.  */
+ mips16_globals/micromips_globals to the PCH file, or to preserve a
+ compression setting across the PCH save and load.  The loading compiler
+ would not have access to the non-GGC parts of mips16_globals or
+ micromips_globals (either from the PCH file, or from a copy that the
+ loading compiler generated itself) and would have to call target_reinit
+ anyway.
+
+ It therefore seems best to switch back to non-MIPS16 mode and
+ non-microMIPS mode to save time, and to ensure that mips16_globals and
+ micromips_globals remain null after a PCH load.  */
   mips_set_compression_mode (0);
   mips16_globals = 0;
+  micromips_globals = 0;
 }
 ^L
 /* Generate or test for an insn that supports a constant permutation.  */




RE: [PATCH,testsuite] MIPS: Downgrade R6 to R5 if tests need branch-likely instructions.

2016-11-03 Thread Moore, Catherine


> -Original Message-
> From: Toma Tabacu [mailto:toma.tab...@imgtec.com]
> Sent: Thursday, November 3, 2016 6:58 AM
> Subject: [PATCH,testsuite] MIPS: Downgrade R6 to R5 if tests need
> branch-likely instructions.
> 
> 
> gcc/testsuite/
> * gcc.target/mips/mips.exp: Add check for -mbranch-likely in
> condition for R5 downgrade.
> 

This patch is OK.
Catherine



RE: [PATCH, testsuite] MIPS: Relax instruction order check in msa-builtins.c.

2016-12-19 Thread Moore, Catherine


> -Original Message-
> From: Toma Tabacu [mailto:toma.tab...@imgtec.com]
> Sent: Thursday, December 15, 2016 9:51 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Matthew Fortune ; Moore,
> Catherine 
> Subject: [PATCH, testsuite] MIPS: Relax instruction order check in msa-
> builtins.c.
> 
> Hi,
> 
> The 32-bit insert.d case in msa-builtins.c is failing with O2 and Os
> because
> the order of the emitted instructions is slightly different compared to
> the
> other optimization levels.
> 
> This patch tweaks the regular expression for 32-bit insert.d to accept
> the
> alternate instruction order.
> 
> Tested with mips-mti-elf.
> 
> Regards,
> Toma
> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.target/mips/msa-builtins.c (dg-final): Tweak regex for the
> 32-bit
>   insert.d case.

Please change to:
* gcc.target/mips-msa-builtins.c (msa_insert_d): Tweak expected output.

Okay with that change.
Thanks,
Catherine

> 
> diff --git a/gcc/testsuite/gcc.target/mips/msa-builtins.c
> b/gcc/testsuite/gcc.target/mips/msa-builtins.c
> index 6db3d66..a679f06 100644
> --- a/gcc/testsuite/gcc.target/mips/msa-builtins.c
> +++ b/gcc/testsuite/gcc.target/mips/msa-builtins.c
> @@ -481,7 +481,7 @@
>  /* { dg-final { scan-assembler-times
> "msa_insert_h:.*insert\\.h.*msa_insert_h" 1 } } */
>  /* { dg-final { scan-assembler-times
> "msa_insert_w:.*insert\\.w.*msa_insert_w" 1 } } */
>  /* { dg-final { scan-assembler-times
> "msa_insert_d:.*insert\\.d.*msa_insert_d" 1 { target mips64 } } } */
> -/* { dg-final { scan-assembler-times
> "msa_insert_d:.*sra.*insert.w.*insert.w.*msa_insert_d" 1 { target {!
> mips64 } } } } */
> +/* { dg-final { scan-assembler
> "msa_insert_d:.*(sra.*insert.w.*insert.w|insert.w.*sra.*insert.w).*ms
> a_insert_d" { target {! mips64 } } } } */
>  /* { dg-final { scan-assembler-times
> "msa_insve_b:.*insve\\.b.*msa_insve_b" 1 } } */
>  /* { dg-final { scan-assembler-times
> "msa_insve_h:.*insve\\.h.*msa_insve_h" 1 } } */
>  /* { dg-final { scan-assembler-times
> "msa_insve_w:.*insve\\.w.*msa_insve_w" 1 } } */



[MIPS, committed] microMIPS testsuite cleanup

2016-05-04 Thread Moore, Catherine

2016-05-04  Kwok Cheung Yeung  

* gcc.target/mips/mips16-attributes.c: Skip if -mmicromips
flag is present.

Index: gcc.target/mips/mips16-attributes.c
===
--- gcc.target/mips/mips16-attributes.c (revision 235880)
+++ gcc.target/mips/mips16-attributes.c (working copy)
@@ -3,6 +3,7 @@
function.  */
 /* { dg-do run } */
 /* { dg-options "(-mips16)" } */
+/* { dg-skip-if "" { *-*-* } { "-mmicromips" } { "" } } */

 #include 



RE: [PATCH][MIPS] Fix register renaming in the interrupt handlers

2015-08-13 Thread Moore, Catherine


> -Original Message-
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Thursday, August 13, 2015 7:20 AM
> To: Robert Suchanek; Moore, Catherine
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH][MIPS] Fix register renaming in the interrupt handlers
> 
> I'd like to give Catherine chance to review this, I notice a couple of 
> formatting
> nits in the test case:

Yes, this patch is OK with Matthew's suggested changes.
Thanks,
Catherine

> 
> Robert Suchanek  writes:
> > 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) {
> 
> Newline for function name and whitespace before args
> 
> > +  if (!foo)
> > +   {
> 
> Double space indent for brace or remove them entirely as it is single
> statement.
> 
> > +  while (bar & 0xFF30);
> > +   }
> > +}
> > +/* { dg-final { scan-assembler-not "\\\$8" } } */
> 
> Thanks,
> Matthew
> 
> 



RE: [RFA] Compact EH Patch

2015-08-14 Thread Moore, Catherine
Hi Jason,
Are you going to be able to review this patch sometime soon?
Thanks,
Catherine

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Moore, Catherine
> Sent: Thursday, July 30, 2015 4:15 PM
> To: gcc-patches@gcc.gnu.org
> Cc: ja...@redhat.com; Matthew Fortune
> Subject: [RFA] Compact EH Patch
> 
> This patch implements a more compact format for exception handling data.
> Although I don't have recent numbers for the amount of compression
> achieved, an earlier measurement showed a 30% reduction in the size of EH
> data for libstdc++.
> 
> A design document detailing the new format is available
> (https://github.com/MentorEmbedded/cxx-
> abi/blob/master/MIPSCompactEH.pdf).
> 
> This implementation enables the new format for MIPS targets only, but the
> generic pieces to enable the new format for other architectures is in place.
> I couldn't really think of a good way to split up the patch to make the review
> easier, but I am open to suggestions.
> I've successfully bootstrapped with the patch and completed testing across
> many of the MIPS targets and multilibs.
> I also ran a test series for the x86 without regressions.
> Thanks,
> Catherine
> 
> 



  1   2   >