RE: [PATCH] Skip re-computing the mips frame info after reload completed

[Matthew Fortune]

Bernd Edlinger  writes:
> this patch skips the redundant re-computing of the frame info after reload 
> completed.
> 
> I looked at all available targets initial_elimination_offset functions:
> 
> All of them currently use either a trivial function of 
> crtl->outgoing_args_size,
> get_frame_size ()
> and df_regs_ever_live_p (x), that can be expected to be evaluated quickly and 
> to be
> constant,
> or they use a cached value, if they are called when reload_completed is true.
> 

Hi Bernd,

For the sake of consistency with other targets then this looks fine. I'd
both prefer the MIPS backend to be as efficient as other targets and not
crash when something like initial elimination offset gets used post
reload. Whether it is right or wrong that we end up with these calls
post-reload is a different matter but I don't understand the detail
enough to comment.

> I believe this patch will both be a performance optimization and
> guarantee that the frame info can not unexpectedly change when it
> should not.

I am a bit dubious about this comment as simply not re-computing the
frame info is not quite the same as it being consistent with
register usage etc. However, we have no check for this right now
post-reload so there is no requirement to improve the situation.

> I have successfully built a cross-compiler with this patch.
> Is it OK for trunk?

Has the patch been tested beyond just building GCC? I can do a
test run for you if you don't have things set up to do one yourself.

Thanks,
Matthew

RE: [PATCH] Skip re-computing the mips frame info after reload completed

[Matthew Fortune]

Bernd Edlinger  writes:
> Matthew Fortune  writes:
> > Has the patch been tested beyond just building GCC? I can do a
> > test run for you if you don't have things set up to do one yourself.
> 
> I built a cross-gcc with all languages and a cross-glibc, but I have
> not set up an emulation environment, so if you could give it a test
> that would be highly welcome.

mipsel-linux-gnu test results are the same before and after this patch.

Please go ahead and commit.

Thanks,
Matthew

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

[Matthew Fortune]

Richard Sandiford  writes:
> "Steve Ellcey "  writes:
> > 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.
> >
> >
> >
> > diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> > index dd54d6a..adeafa3 100644
> > --- a/gcc/config/mips/mips.c
> > +++ b/gcc/config/mips/mips.c
> > @@ -8006,9 +8006,18 @@ mask_low_and_shift_p (machine_mode mode, rtx mask, 
> > rtx shift, int
> maxlen)
> >  bool
> >  and_operands_ok (machine_mode mode, rtx op1, rtx op2)
> >  {
> > -  return (memory_operand (op1, mode)
> > - ? and_load_operand (op2, mode)
> > - : and_reg_operand (op2, mode));
> > +
> > +  if (memory_operand (op1, mode))
> > +{
> > +  if (TARGET_MIPS16) {
> > +   struct mips_address_info addr;
> > +   if (!mips_classify_address (&addr, op1, mode, false))
> > + return false;
> > +  }
> 
> Nit: brace formatting.
> 
> It looks like the patch only works by accident.  The code above
> is passing the MEM, rather than the address inside the MEM, to
> mips_classify_address.  Since (mem (mem ...)) is never valid on MIPS,
> the effect is to disable the memory alternatives of *and3_mips16
> unconditionally.
> 
> The addresses that occur in the testcase are valid as far as
> mips_classify_address is concerned.  FWIW, there shouldn't be any
> difference between the addresses that memory_operand accepts and the
> addresses that mips_classify_address accepts.
> 
> In theory, the "W" constraints in *and3_mips16 are supposed to
> tell the target-independent code that this instruction cannot handle
> constant addresses or stack-based addresses.  That seems to be working
> correctly during RA for the testcase.  The problem comes in regcprop,
> which ends up creating a second stack pointer rtx distinct from
> stack_pointer_rtx:
> 
> (reg/f:SI 29 $sp [375])
> 
> (Note the ORIGINAL_REGNO of 375.)  This then defeats the test in
> mips_stack_address_p:
> 
> bool
> mips_stack_address_p (rtx x, machine_mode mode)
> {
>   struct mips_address_info addr;
> 
>   return (mips_classify_address (&addr, x, mode, false)
> && addr.type == ADDRESS_REG
> && addr.reg == stack_pointer_rtx);
> }
> 
> Change the == to rtx_equal_p and the test passes.  I don't think that's
> the correct fix though -- the fix is to stop a second stack pointer rtx
> from being created.

Agreed, though I'm inclined to say do both. We actually hit this
same issue while testing some 4.9.2 based tools recently but I hadn't
got confirmation from Andrew (cc'd) whether it was definitely the same
issue. Andrew fixed this by updating all tests against stack_pointer_rtx
to compare register numbers instead (but rtx_equal_p is better still).

I think it is worthwhile looking in more detail why a new stack pointer
rtx appears. Andrew did look briefly at the time but I don't recall his
conclusions...

Matthew

RE: Remove -fshort-double (PR60410)

[Matthew Fortune]

Jeff Law  writes:
> On 02/05/2016 12:31 PM, Bernd Schmidt wrote:
> > This patch fixes PR60410 by removing -fshort-double. Nick earlier
> > propsed a fix for the crash, but Richard B suggested removing the option
> > entirely, and I'd agree with that. It's a pointless ABI-changing option
> > on most targets, and if a port really needs it, it should be a -m option
> > that tweaks DOUBLE_TYPE_SIZE.
> >
> > It turns out that there is still a mips config that enables it for a set
> > of multilibs. As mentioned here:
> >https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02140.html
> > I've not managed to make that config build, it fails configuring libgcc.
> > A mips maintainer would have to speak up as to whether that config is
> > useful at all or not.

Sorry for being slow to reply.

This configuration was added to MIPS as one way to make efficient use of
a single precision only FPU. The intention was to avoid promotion to
doubles where the calling convention would normally require it. The
prime example being floating point varargs arguments. There are however
a huge amount of failures in this mode when testing but it was good enough
for some users. Given there is an alternative then I have no objection
to removing it. It will be a couple of days before I have chance to
implement a -m option and update the mips-img-elf multilib.

Thanks,
Matthew

RE: [MIPS r5900] libgcc floating point fixes

[Matthew Fortune]

Woon yung Liu  wries
> Bump! Sorry, but could I please get an answer? I'm willing to update the 
> patch without
> credit, if necessary.

Hi WY,

Apologies for exceptionally slow response.

The patch you referenced is mostly OK but I would like to get the MIPS16 check
changed to a configure time check for MIPS16 support rather than checking for
r5900. I.e. I think we should have GCC raise an error for -march=r5900 -mips16
and then a configure time check using just -mips16 would fail. That can then
be used to choose whether to build the mips16 code instead of this:

+   if test x$with_arch != xr5900; then
+   tmake_file="$tmake_file mips/t-mips16"
+   fi

This change should also make it possible to have mips.exp simply skip the mips16
tests for r5900 without having to tell it explicitly about r5900.

Thanks,
Matthew

> The patch is working for the R5900 hard-fp mode. I've also used the same, 
> patched copy of
> GCC, to build the toolchain for the IOP (MIPS R3000A, 32-bit MIPS I with no 
> FPU) and it
> also builds correctly.
> 
> If I should be writing to someone else specifically, could someone please 
> tell me who I
> should be writing to instead?
> 
> 
> Thanks and regards,
> -W Y
> 
> 
> 
> On Tuesday, January 26, 2016 5:41 PM, Woon yung Liu  wrote:
> Hi,
> 
> I refer to the previous message by Juergen, regarding his patch to libgcc.
> https://gcc.gnu.org/ml/gcc-patches/2014-08/msg01725.html
> 
> As of now, libgcc (of GCC v5.3.0) still has the problem of building support 
> for both soft
> and hard floats, when there is no support for hard floats by the R5900 (and 
> hence
> resulting in the generation of recursive functions like extendsfdf2).
> 
> That patch doesn't seem to have been committed. I would very much like to 
> help to see it
> get committed because GCC's support for the R5900 is currently not suitable 
> for
> PlayStation 2 development; software-floating point emulation is severely 
> detrimental to
> performance.
> What else needs to be done first, before it can be accepted?
> 
> Thanks and regards,
> -W Y

RE: [Patch, MIPS] Patch for PR 68273 (user aligned variable arguments)

[Matthew Fortune]

Steve Ellcey   writes:
> Here is a new patch for PR 68273.  The original problem with gsoap has
> been fixed by changing GCC to not create overly-aligned variables in
> the SRA pass but the MIPS specific problem of how user-aligned variables
> are passed to functions remains.
> 
> Because MIPS GCC is internally inconsistent, it seems like changing
> the passing convention for user aligned variables on MIPS is the best
> option, even though this is an ABI change for MIPS.
> 
> For example:
> 
>   typedef int alignedint __attribute__((aligned(8)));
>   int foo1 (int x, alignedint y) { return x+y; }
>   int foo2 (int x, int y) { return x+y; }
>   int foo3 (int x, alignedint y) { return x+y; }
>   int foo4 (int x, int y) { return x+y; }
>   int splat1(int a, alignedint b) { foo1(a,b); }
>   int splat2(int a, alignedint b) { foo2(a,b); }
>   int splat3(int a, int b) { foo3(a,b); }
>   int splat4(int a, int b) { foo4(a,b); }
> 
> In this case foo1 and foo3 would expect the second argument to be in
> register $6, but foo2 and foo4 would exect it in register $5.
> 
> Likewise splat1 and splat2 would pass the second argument in $6, but
> splat3 and splat4 would pass it in $5.
> 
> This means that the call from splat2 to foo2 and the call from splat3
> to foo3 would be wrong in that the caller is putting the argument in
> one register but the callee is getting out of a different register.

Thanks for enumerating all the cases. I'd not looked at all of them. I
do agree that we need a fix given the existing inconsistencies.

One question I have is where does an over aligned argument get pushed
to the stack with the patch in place. I.e. when taking its address, is
the alignment achieved up to the limit of stack alignment or do they now
only get register-size alignment? If the former then the idea that
argument passing is defined as a structure in memory with the first
portion in registers will no longer hold true. Not sure if that is a
problem.

> In none of these cases would GCC give a warning about the inconsistent
> parameter passing
> 
> Since this patch could cause a change in the users program, I have
> added a warning that will be emitted whenever a user passes an
> aligned type or when a parameter is declared as an aligned type
> and that alignment could cause a change in how the value is passed.
> 
> I also added a way to turn that warning off in case the user doesn't
> want to see it (-mno-warn-aligned-args).  I did not add an option
> to make GCC pass arguments in the old manner as I consider that method
> of passing arguments to be a bug and I don't think we want to have an
> option to propogate that incorrect behavior.

This sounds good to me.

I'd like to wait and see if anyone else has comments here given the
amount of discussion there has been on this PR.

I've pointed out a couple of style issues inline below.

Matthew

> 
> Steve Ellcey
> sell...@imgtec.com
> 
> 
> 2016-02-24  Steve Ellcey  
> 
>   PR target/68273
>   * config/mips/mips.opt (mwarn-aligned-args): New flag.

Needs doc/invoke.texi update.

>   * config/mips/mips.h (mips_args): Add new field.
>   * config/mips/mips.c (mips_internal_function_arg): New function.
>   (mips_function_incoming_arg): New function.
>   (mips_old_function_arg_boundary): New function.
>   (mips_function_arg): Rewrite to use mips_internal_function_arg.
>   (mips_function_arg_boundary): Fix argument alignment.
>   (TARGET_FUNCTION_INCOMING_ARG): New define.
> 
> 
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index 697abc2..05465c1 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -1124,6 +1124,7 @@ static const struct mips_rtx_cost_data
>  static rtx mips_find_pic_call_symbol (rtx_insn *, rtx, bool);
>  static int mips_register_move_cost (machine_mode, reg_class_t,
>   reg_class_t);
> +static unsigned int mips_old_function_arg_boundary (machine_mode,
> const_tree);
>  static unsigned int mips_function_arg_boundary (machine_mode,
> const_tree);
>  static machine_mode mips_get_reg_raw_mode (int regno);
>  
> @@ -5459,11 +5460,11 @@ mips_strict_argument_naming (cumulative_args_t
> ca ATTRIBUTE_UNUSED)
>return !TARGET_OLDABI;
>  }
> 
> -/* Implement TARGET_FUNCTION_ARG.  */
> +/* Used to implement TARGET_FUNCTION_ARG and
> TARGET_FUNCTION_INCOMING_ARG.  */
> 
>  static rtx
> -mips_function_arg (cumulative_args_t cum_v, machine_mode mode,
> -const_tree type, bool named)
> +mips_internal_function_arg (cumulative_args_t cum_v, machine_mode mode,
> + const_tree type, bool named)
>  {
>CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);
>struct mips_arg_info info;
> @@ -5586,6 +5587,50 @@ mips_function_arg (cumulative_args_t cum_v,
> machine_mode mode,
>return gen_rtx_REG (mode, mips_arg_regno (&info, TARGET_HARD_FLOAT));
>  }
> 
> +/* Implement TARGET_FUNCTION_ARG.  */
> +
> 

RE: [Patch, MIPS] Fix SYSROOT_SUFFIX_SPEC for mips-mti-linux-gnu

[Matthew Fortune]

> 2015-07-09  Steve Ellcey  
> 
>   * config/mips/mti-linux.h (MIPS_SYSVERSION_SPEC): Update
>   to handle mips[32|64]r3 and mips[32|64]r5.

OK, thanks.

Matthew

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

[Matthew Fortune]

Andrew Bennett  writes:
> I have noticed that in the mips.exp dg-option handling code the isa and
> arch_test_option_p variables are not updated after the pre-arch to arch
> dependency handling.  This means that if this code changes the
> architecture the post-arch dependency handling code (which relies on
> arch_test_option_p being true) is not run to handle any extra dependencies
> the new architecture might need.

I'm not sure this is the right place to fix this, though it does seem
subjective as we are stretching the logic a little I think.

In the pre-arch options (i.e. when an arch is not explicitly requested) we
already have code that sets -mnan-2008 when downgrading a test R6 to R5 as
the R6 headers will be nan2008 and there is no guarantee of nan legacy headers
existing. This is the opposite case where we upgrade a test from R5 to R6
and R6 has to use -mnan=2008 so needs to explicitly override any command line
option to use -mnan=legacy. I think that therefore needs adding when we set
the arch to R6 in the pre-arch options.

At the same time I think we need to add -mabs=2008 in the same place as R6
requires ABS2008 as well. You should see that as a failure if you test with
-mabs=legacy.

I think I wrote the exact same patch as you have when I did the original R6
tests and concluded it was not in-keeping with the structure of mips.exp.

I've added Richard too since he may be able to offer a guiding hand as original
author of most of the mips.exp code.

Thanks,
Matthew

> I have found this issue while investigating failures with the mips-mti-elf
> toolchain using the -mnan=legacy multilib flags when running any of the
> mips tests that have the HAS_LSA option specified in the dg-options.  The
> default architecture for this toolchain is mips32r2.  This means the 
> architecture
> handling code changes the architecture to mips32r6 to handle the HAS_LSA
> requirements.  Unfortunately because the arch_test_option_p is not updated
> it is still set to false, so the post-arch code is not run.  This means
> the nan encoding is not set to -mnan=2008 when then causes the tests to fail
> because mips32r6 does not support -mnan=legacy.
> 
> The patch and ChangeLog are below.
> 
> Ok to commit?
> 
> 
> 
> Regards,
> 
> 
> 
> Andrew
> 
> 
> testsuite/
>   * gcc.target/mips/mips.exp (mips-dg-options): Update the isa and
>   arch_test_option_p variables after the arch dependency handling code.
> 
> 
> diff --git a/gcc/testsuite/gcc.target/mips/mips.exp
> b/gcc/testsuite/gcc.target/mips/mips.exp
> index 1dd4173..1eb714d 100644
> --- a/gcc/testsuite/gcc.target/mips/mips.exp
> +++ b/gcc/testsuite/gcc.target/mips/mips.exp
> @@ -1188,8 +1188,10 @@ proc mips-dg-options { args } {
>  }
> 
>  # Re-calculate the isa_rev for use in the abi handling code below
> +set arch_test_option_p [mips_test_option_p options arch]
>  set arch [mips_option options arch]
>  set isa_rev [mips_arch_info $arch isa_rev]
> +set isa [mips_arch_info $arch isa]
> 
>  # Set an appropriate ABI, handling dependencies between the pre-abi
>  # options and the abi options.  This should mirror the abi and post-abi

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

[Matthew Fortune]

Robert Suchanek  writes:
> > Hi Robert,
> > The patch is OK, but will you please name the test something other than the
> > date?
> 
> OK. I'll change it to interrupt_handler-5.c, add a comment and commit after
> approval for the new interrupt handler options.

I believe this change is independent of the new attributes so feel free to 
commit
it before.

Thanks,
Matthew

RE: [PATCH, MIPS] Scheduling for M51xx core family

[Matthew Fortune]

Richard Sandiford  writes:
> Robert Suchanek  writes:
> > @@ -771,7 +771,8 @@ struct mips_cpu_info {
> >
> >  /* Infer a -mnan=2008 setting from a -mips argument.  */
> >  #define MIPS_ISA_NAN2008_SPEC \
> > -  "%{mnan*:;mips32r6|mips64r6:-mnan=2008}"
> > +  "%{mnan*:;mips32r6|mips64r6:-mnan=2008;march=m51*: \
> > +%{!msoft-float:-mnan=2008}}"
> 
> Did you need this, or was it for completeness?  MIPS_ISA_NAN2008_SPEC
> should only be used after MIPS_ISA_LEVEL_SPEC, so I would have expected
> the mips32r6|mips64r6: case to fire for -march=m51* too, ahead of the
> new case.

The m5100 is a MIPS32R5 but with a NAN2008 FPU which is why there is the
special case. The soft-float case is to try and limit the number of
multilib variants required so we stick to nan legacy for softfloat by
default.

Thanks,
Matthew

RE: [PATCH, MIPS] Scheduling for M51xx core family

[Matthew Fortune]

Robert Suchanek  writes.
> 2015-07-16  Prachi Godbole  
> 
> gcc/
> 
>   * config/mips/m5100.md: New file.
>   * config/mips/mips-cpus.def (m5100, m5101): Define.
>   * config/mips/mips-tables.opt: Regenerate.
>   * config/mips/mips.c (mips_rtx_cost_data): Add costs for m5100.
>   * config/mips/mips.h (MIPS_ISA_LEVEL_SPEC): Map -march=m5100 and
>   -march=m5101 to -mips32r5.
>   (MIPS_ARCH_FLOAT_SPEC): Map -m5101 to -msoft-float.
>   (MIPS_ISA_NAN2008_SPEC): Map -march=m51* to -mnan=2008 if
>   !-msoft-float.
>   * config/mips/mips.md: Include m5100.md.
>   (processor): Add m5100.
>   * doc/invoke.texi (-march=@var{arch}): Add m5100, m5101.

OK, this looks fine.

I did realise while reading through this that the MIPS_ARCH_FLOAT_SPEC
is not used for and ordinary MIPS Linux compiler which seems odd but
I presume this is to make it possible to use one hard-float sysroot
for any core and emulate the FPU when not present.

I think it is probably a mistake to have put MIPS_ARCH_FLOAT_SPEC in
the mti-linux.h and android.h DRIVER_SELF_SPECS so I think they need
removing. Although we support building soft-float multilibs I don't
think they actually get used very much so leaving the selection of
soft-float down to the end user in Linux seems wise.

With the i6400 scheduler committed then we can also get rid of the w32
and w64 placeholders that were there solely to provide an R6 processor
to use as the default processor for the generic arch options.

Thanks,
Matthew

RE: [PATCH, MIPS] I6400 scheduling

[Matthew Fortune]

Robert Suchanek  writes:
> 2015-07-16  Prachi Godbole  
> 
> gcc/
>   * config/mips/i6400.md: New file.
>   * config/mips/mips-cpus.def (mips32r6): Change to PROCESSOR_I6400.
>   (mips64r6): Likewise.
>   (i6400): Define.
>   * config/mips/mips-tables.opt: Regenerate.
>   * config/mips/mips.c (mips_rtx_cost_data): Add I6400 processor.
>   (mips_issue_rate): Add support for i6400.
>   (mips_multipass_dfa_lookahead): Likewise.
>   * config/mips/mips.h (TUNE_I6400): Define.
>   * config/mips/mips.md: Include i6400.md.
>   (processor): Add i6400.
>   * doc/invoke.texi (-march=@var{arch}): Add i6400.

Apologies, I thought Catherine had done this one but not the M5100. Just
a couple of things...

> ---
>  gcc/config/mips/i6400.md| 142
> 
>  gcc/config/mips/mips-cpus.def   |   7 +-
>  gcc/config/mips/mips-tables.opt |   3 +
>  gcc/config/mips/mips.c  |  16 -
>  gcc/config/mips/mips.h  |   3 +-
>  gcc/config/mips/mips.md |   2 +
>  gcc/doc/invoke.texi |   1 +
>  7 files changed, 170 insertions(+), 4 deletions(-)  create mode 100644
> gcc/config/mips/i6400.md
> 
> diff --git a/gcc/config/mips/i6400.md b/gcc/config/mips/i6400.md new
> file mode 100644 index 000..101a20c
> --- /dev/null
> +++ b/gcc/config/mips/i6400.md
> @@ -0,0 +1,142 @@
> +;; DFA-based pipeline description for I6400.
> +;;
> +;; Copyright (C) 2007-2015 Free Software Foundation, Inc.

This should just be 2015.

> diff --git a/gcc/config/mips/mips-cpus.def b/gcc/config/mips/mips-
> cpus.def index fb4bae0..90836a3 100644
> --- a/gcc/config/mips/mips-cpus.def
> +++ b/gcc/config/mips/mips-cpus.def
> @@ -50,13 +50,13 @@ MIPS_CPU ("mips32r2", PROCESSOR_74KF2_1, 33,
> PTF_AVOID_BRANCHLIKELY)
> as mips32r2.  */
>  MIPS_CPU ("mips32r3", PROCESSOR_M4K, 34, PTF_AVOID_BRANCHLIKELY)
> MIPS_CPU ("mips32r5", PROCESSOR_P5600, 36, PTF_AVOID_BRANCHLIKELY) -
> MIPS_CPU ("mips32r6", PROCESSOR_W32, 37, PTF_AVOID_BRANCHLIKELY)
> +MIPS_CPU ("mips32r6", PROCESSOR_I6400, 37, PTF_AVOID_BRANCHLIKELY)
>  MIPS_CPU ("mips64", PROCESSOR_5KC, 64, PTF_AVOID_BRANCHLIKELY)
>  /* ??? For now just tune the generic MIPS64r2 and above for 5KC as
> well.   */
>  MIPS_CPU ("mips64r2", PROCESSOR_5KC, 65, PTF_AVOID_BRANCHLIKELY)
> MIPS_CPU ("mips64r3", PROCESSOR_5KC, 66, PTF_AVOID_BRANCHLIKELY)
> MIPS_CPU ("mips64r5", PROCESSOR_5KC, 68, PTF_AVOID_BRANCHLIKELY) -
> MIPS_CPU ("mips64r6", PROCESSOR_W64, 69, PTF_AVOID_BRANCHLIKELY)
> +MIPS_CPU ("mips64r6", PROCESSOR_I6400, 69, PTF_AVOID_BRANCHLIKELY)
> 
>  /* MIPS I processors.  */
>  MIPS_CPU ("r3000", PROCESSOR_R3000, 1, 0) @@ -166,3 +166,6 @@ MIPS_CPU
> ("octeon+", PROCESSOR_OCTEON, 65, PTF_AVOID_BRANCHLIKELY)  MIPS_CPU
> ("octeon2", PROCESSOR_OCTEON2, 65, PTF_AVOID_BRANCHLIKELY)  MIPS_CPU
> ("octeon3", PROCESSOR_OCTEON3, 65, PTF_AVOID_BRANCHLIKELY)  MIPS_CPU
> ("xlp", PROCESSOR_XLP, 65, PTF_AVOID_BRANCHLIKELY)
> +
> +/* MIPS64 Release 6 processors.  */
> +MIPS_CPU ("i6400", PROCESSOR_I6400, 69, PTF_AVOID_BRANCHLIKELY)

I don't think this really matters but the PTF_AVOID_BRANCHLIKELY should
not be necessary for R6 cores as there are no branch likely instructions.
Changing this may also require an update to the option handling code
in mips.c I don't know if it will try to enable branch likely if you
remove this.

OK with those changes.

Does the I6400 support load/store bonding? I seem to think it does but
could be wrong. If it does then dealing with it in a follow up patch is
OK with me.

Thanks,
Matthew

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

[Matthew Fortune]

Andrew Bennett  writes:
> 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.

Thanks,
Matthew

[PATCH, MIPS] Compact branch support for MIPS32R6/MIPS64R6

[Matthew Fortune]

A full range of 'compact' branch instructions were introduced to MIPS
as part of Release 6. The compact term is used to identify the fact
that these do not have a delay slot.

http://imgtec.com/mips/architectures/mips64/

The one subtlety of compact branches is that while they do not have
a delay slot they do have a restriction on what can immediately follow
them. The restriction is referred to as a forbidden slot in the
architecture specification and exists only on the not-taken path of
a conditional compact branch. (The detail of whether the hazard exists
on a not-taken branch is not relevant to a compiler however as it
has to be accounted for anyway as we would not generate a compact
branch if it were always taken.)

The forbidden slot restriction equates to the same rule as delay slots
where control flow instructions are not allowed to be placed there. The
exact same set of instructions cannot be placed in a forbidden slot.

An additional class of branch instructions is also available in
compact form only which allow ordering conditions to be applied
between two register sources. Support for these is included in this
patch.

So how does all this work in GCC?

Compact branches are used based on a branch policy. The polices are:

never: Only use delay slot branches
optimal: Do whatever is best for the current architecture.  This will
 generally mean that delay slot branches will be used if the delay
 slot gets filled but otherwise a compact branch will be used. A
 special case here is that JAL and J will not be used in R6 code
 regardless of whether the delay slot could be filled.
always: Never emit a delay slot form of a branch if a compact form exists.
This policy cannot apply 100% as FP branches (and MSA branches when
committed) only have delay slot forms.

These user choices are combined with the features available in the chosen
architecture and, in particular, the optimal form will get handled like
'never' when there are no compact branches available and will get handled
like 'always' when there are no delay slot branches available.

>From an instruction description perspective we also mark each branch with
a compact_form attribute that says if it 'never' has a compact form, 'maybe'
has a compact form dependent on delay slot filling, or 'always' comes in
a compact form. A secondary attribute is also used to describe whether the
instruction has a forbidden slot hazard. This applies to conditional compact
branches and means that although they do not have a delay slot, it is still
not possible to place a branch instruction immediately after them.

The define_delay definitions are configured by a combination of the user
selected branch policy and the compact_form attribute. This means the
delay slot filler will only operate on branches that should have delay slots.

Output patterns for branches fall into two categories:

1) Predetermined to be compact or delay slot, or this has been detected at
   the point of emitting the pattern. These will generally not use any
   formatters for the 'c' or the trailing NOP that normally get automatically
   injected by the mips_print_operand_punctuation function.
2) Use instruction formatters to enable a branch to naturally become a
   delay slot or compact form depending on whether a delay slot has been
   filled. These will use the %: formatter to indicate that a 'c' can
   be added instead of inserting a NOP using the %/ formatter. I.e.
   %: and %/ should never appear in the same branch instruction pattern.

It is generally safe to rely on using the formatters to produce the correct
branch instructions as a branch instruction that can only have a compact
form will not have a define_delay and therefore will never be in a final
sequence... This then means the %: is guaranteed to emit a 'c'.

The most complicated aspect of this change is to the MIPS_CALL and
MICROMIPS_J macros. These have been rewritten from scratch as a function
that generates an instruction instead.  This code is more complicated than
ordinary 'branch' code as J becomes BC and JAL become BALC which renders
instruction formatters impossible to use. The complexities of pic/non-pic
microMIPS/MIPS and absolute/relative addressing meant that wrapping all
that up in one place made much more sense. Matching the old macros to the
new function is hard but the conversion has been done carefully with a
significant amount of focussed testing.

Some of the framework in this patch is there in preparation for microMIPSR6
which only has compact branches. The support for adding microMIPSR6 to
GCC is a trivial patch on top of this.

This has been tested on multiple configurations albeit that most
configurations were tested from an older trunk revision. A re-run of a
wide range of configurations will be done after review/before commit.
This code has also been in use as part of tools to support internal
development of the I6400 core from Imagination.

gcc/
* con

RE: [PATCH] Fix undefined behaviour in mips port

[Matthew Fortune]

Jeff Law  writes:
> Another instance of left shifting a negative value.  Fixed in an obvious
> way.  Verified all the mips configurations in config-list.mk build now
> using a trunk compiler.
> 
> Installed on the trunk,

Thanks Jeff.

Matthew

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

[Matthew Fortune]

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 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 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 1/4] [MIPS] Add support for MIPS SIMD Architecture (MSA)

[Matthew Fortune]

Hi Robert,

Next batch of comments. This set covers the rest of mips-msa.md.

>+++ b/gcc/config/mips/mips-msa.md
>+(define_expand "vec_perm"
>+  [(match_operand:MSA 0 "register_operand")
>+   (match_operand:MSA 1 "register_operand")
>+   (match_operand:MSA 2 "register_operand")
>+   (match_operand: 3 "register_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  /* The optab semantics are that index 0 selects the first element
>+ of operands[1] and the highest index selects the last element
>+ of operands[2].  This is the oppossite order from "vshf.df wd,rs,wt"
>+ where index 0 selects the first element of wt and the highest index
>+ selects the last element of ws.  We therefore swap the operands here.  */
>+  emit_insn (gen_msa_vshf (operands[0], operands[3], operands[2],
>+   operands[1]));
>+  DONE;
>+})

Can you make this the real instruction instead of msa_vshf and give it a
proper pattern (vec_select, vec_concat) etc. Swap the builtin to target
this pattern and swap the operands for the builtin expansion in C code
like you have done for some other patterns.

>+(define_expand "neg2"
>+  [(match_operand:IMSA 0 "register_operand")
>+   (match_operand:IMSA 1 "register_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  rtx reg = gen_reg_rtx (mode);
>+  emit_insn (gen_msa_ldi (reg, const0_rtx));
>+  emit_insn (gen_sub3 (operands[0], reg, operands[1]));
>+  DONE;
>+})
>+
>+(define_expand "neg2"
>+  [(match_operand:FMSA 0 "register_operand")
>+   (match_operand:FMSA 1 "register_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  rtx reg = gen_reg_rtx (mode);
>+  emit_move_insn (reg, CONST0_RTX (mode));
>+  emit_insn (gen_sub3 (operands[0], reg, operands[1]));
>+  DONE;
>+})

Can't these two collapse into one like this?

(define_expand "neg2"
  [(set (match_operand:MSA 0 "register_operand")
(minus:MSA (match_dup 2)
   (match_operand:MSA 1 "register_operand")))]
  "ISA_HAS_MSA"
{
  operands[2] = CONST0_RTX (mode);
})

I'd hope the const_vector then gets emitted as an LDI? I haven't
checked that there is a pattern for using LDI for FP const_vector
moves.

>+(define_expand "msa_ldi"
>+  [(match_operand:IMSA 0 "register_operand")
>+   (match_operand 1 "const_imm10_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  unsigned n_elts = GET_MODE_NUNITS (mode);
>+  rtvec v = rtvec_alloc (n_elts);
>+  HOST_WIDE_INT val = INTVAL (operands[1]);
>+  unsigned int i;
>+
>+  if (mode != V16QImode)
>+{
>+  unsigned shift = HOST_BITS_PER_WIDE_INT - 10;
>+  val = trunc_int_for_mode ((val << shift) >> shift, mode);
>+}
>+  else
>+val = trunc_int_for_mode (val, mode);
>+
>+  for (i = 0; i < n_elts; i++)
>+RTVEC_ELT (v, i) = GEN_INT (val);
>+  emit_move_insn (operands[0],
>+gen_rtx_CONST_VECTOR (mode, v));
>+  DONE;
>+})

This is really weird. We shouldn't be simply discarding bits that don't fit.
This needs to accept all immediates and generate the correct code to
get a replicated constant of that value into a register. I think it is
probably OK to trunc_int_for_mode on the original 'val' for the
mode but anything out of range for V*HI/SI/DI needs to be expanded
properly.

Please do not gen_msa_ldi anywhere other than from MSA builtins. There is
no need just emit a move directly.

>+(define_insn "msa_vshf"
>+  [(set (match_operand:MSA 0 "register_operand" "=f")
>+  (unspec:MSA [(match_operand: 1 "register_operand" "0")
>+   (match_operand:MSA 2 "register_operand" "f")
>+   (match_operand:MSA 3 "register_operand" "f")]
>+  UNSPEC_MSA_VSHF))]
>+  "ISA_HAS_MSA"
>+  "vshf.\t%w0,%w2,%w3"
>+  [(set_attr "type" "simd_sld")
>+   (set_attr "mode" "")])

Delete this and switch to using vec_perm directly instead for the builtin.

>+;; 128bit MSA modes only in msa registers or memory.  An exception is allowing

128-bit MSA modes can only exist in MSA registers or memory. ...

>+;; Offset load
>+(define_expand "msa_ld_"
>+  [(match_operand:MSA 0 "register_operand")
>+   (match_operand 1 "pmode_register_operand")
>+   (match_operand 2 "aq10_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  rtx addr = plus_constant (GET_MODE (operands[1]), operands[1],
>+INTVAL (operands[2]));
>+  mips_emit_move (operands[0], gen_rtx_MEM (mode, addr));
>+  DONE;
>+})
>+
>+;; Offset store
>+(define_expand "msa_st_"
>+  [(match_operand:MSA 0 "register_operand")
>+   (match_operand 1 "pmode_register_operand")
>+   (match_operand 2 "aq10_operand")]
>+  "ISA_HAS_MSA"
>+{
>+  rtx addr = plus_constant (GET_MODE (operands[1]), operands[1],
>+  INTVAL (operands[2]));
>+  mips_emit_move (gen_rtx_MEM (mode, addr), operands[0]);
>+  DONE;
>+})

There's no real need to expand these in C code. The patterns can be used
to create the RTL. As an aside, I don't really see the point in intrinsics
to load and store data the same thing can be done from straight C. 
The patterns also can't be used for const or volatile data as their
built

RE: [PATCH, mips]: Use ROUND_UP and ROUND_DOWN macros

[Matthew Fortune]

Uros Bizjak  writes:
> Fairly trivial patch that introduces no functional changes.
> 
> * config/mips/mips.h (MIPS_STACK_ALIGN): Implement using
> ROUND_UP macro.
> * config/mips/mips.c (mips_setup_incoming_varargs): Use
> ROUND_DOWN to calculate off.
> (mips_gimplify_va_arg_expr): Use ROUND_UP to calculate rsize.
> (mips_emit_probe_stack_range): Use ROUND_DOWN to calculate
> rounded_size.
> 
> Tested by building a crosscompiler to powerpc64-linux-gnu.
> 
> OK for mainline?

OK assuming you meant mips64-linux-gnu or some MIPS triple?

Thanks,
Matthew

RE: [RFA] Compact EH Patch

[Matthew Fortune]

> 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

Hi Catherine,

I'd be lying if I said I understand all of this but I follow the bulk of what 
is needed
from the MIPS side of things. This looks fine to me. There is some code which 
may not
be MIPS specific in here, a request to change the magic constants to macros and 
comments
in the key function that actually generates compact EH entries 
(mips_cfi_endproc).

As this seems quite well separated from non-compact-eh cases then I am more 
than happy
to deal with any issues or refinement incrementally. I'd like to get such a big 
patch
committed sooner rather than later.

Comments inline.

Thanks,
Matthew

>Index: libgcc/config/mips/mips-unwind.h
>===
>--- libgcc/config/mips/mips-unwind.h   (revision 0)
>+++ libgcc/config/mips/mips-unwind.h   (revision 0)
>@@ -0,0 +1,182 @@
>+/* Compact EH unwinding support for MIPS.
>+   Copyright (C) 2012-2015 Free Software Foundation, Inc.
>+
>+This file is part of GCC.
>+
>+GCC is free software; you can redistribute it and/or modify
>+it under the terms of the GNU General Public License as published by
>+the Free Software Foundation; either version 3, or (at your option)
>+any later version.
>+
>+GCC is distributed in the hope that it will be useful,
>+but WITHOUT ANY WARRANTY; without even the implied warranty of
>+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+GNU General Public License for more details.
>+
>+Under Section 7 of GPL version 3, you are granted additional
>+permissions described in the GCC Runtime Library Exception, version
>+3.1, as published by the Free Software Foundation.
>+
>+You should have received a copy of the GNU General Public License and
>+a copy of the GCC Runtime Library Exception along with this program;
>+see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>+.  */
>+
>+#ifdef MD_HAVE_COMPACT_EH
>+
>+#define DWARF_SP_REGNO 29
>+
>+#if _MIPS_SIM == _ABIO32
>+#define MIPS_EH_STACK_ALIGN 8
>+#else
>+#define MIPS_EH_STACK_ALIGN 16
>+#endif
>+
>+#define VRF_0 32
>+

The following two functions are missing a brief comment

>+static int
>+record_push (_Unwind_FrameState *fs, int reg, int offset)
>+{
>+  int idx = DWARF_REG_TO_UNWIND_COLUMN (reg);
>+
>+  offset -= dwarf_reg_size_table[idx];
>+  fs->regs.reg[idx].how = REG_SAVED_OFFSET;
>+  fs->regs.reg[idx].loc.offset = offset;
>+  return offset;
>+}
>+
>+static void
>+record_cfa_adjustment (_Unwind_FrameState *fs, _uleb128_t val)
>+{
>+  int i;
>+  fs->regs.cfa_offset += val;
>+  /* In case we see an adjustment after pushes, it means that
>+ the registers aren't saved at the top of the frame, probably
>+ due to a varargs save area.  Adjust the recorded offsets.  */
>+  for (i = 0; i < DWARF_FRAME_REGISTERS + 1; i++)
>+if (fs->regs.reg[i].how == REG_SAVED_OFFSET)
>+  fs->regs.reg[i].loc.offset -= val;
>+}
>+
>+/* Process the frame unwinding opcodes.  */
>+
>+static _Unwind_Reason_Code
>+md_unwind_compact (struct _Unwind_Context *context ATTRIBUTE_UNUSED,
>+ _Unwind_FrameState *fs, const unsigned char **pp)
>+{
>+  unsigned char op;
>+  _uleb128_t val;
>+  int push_offset;
>+  int i;
>+  int n;
>+  const unsigned char *p = *pp;
>+
>+  push_offset = 0;
>+  fs->regs.cfa_how = CFA_REG_OFFSET;
>+  fs->regs.cfa_reg = STACK_POINTER_REGNUM;
>+  fs->regs.cfa_offset = 0;
>+  fs->retaddr_column = 31;
>+
>+  while (1)
>+{
>+  op = *(p++);
>+
>+  if (op < 0x40)

Can we get these magic constants converted to some macros? I think there are 
sensible
names they could be given.

>+  {
>+/* Increment stack pointer.  */
>+record_cfa_adjustment (fs, (op + 1) * MIPS_EH_STACK_ALIGN);
>+  }
>+  else if (op < 0x48)
>+  {
>+/* Push VR[16] to VR[16+x] and VR[31] */
>+push_offset = record_push (fs, 31, push_offset);
>+for (i = op & 7; i >= 0; i--)
>+  push_offset = record_push (fs, 16 + i, push_offset);
>+  }
>+  else if (op < 0x50)
>+  {
>+/* Push VR[16] to VR[16+x], VR[30] and VR[31] */
>+push_offset = r

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

[Matthew Fortune]

Maciej Rozycki  writes:
> 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?

OK, thanks.

Matthew

RE: [PATCH] MIPS16/GCC: Emit bounds checking as RTL in `casesi'

[Matthew Fortune]

Maciej Rozycki  writes:
>  Further to my changes made last November here is an update to the MIPS16
> `casesi' pattern making it emit bounds checking as RTL rather than having
> it as hardcoded assembly within the `casesi_internal_mips16_'
> dispatcher.  See below for how PR tree-optimization/51513 has prevented me
> from proceeding back then.
> 
>  This new arrangement has several advantages:
> 
> 1. There is no hardcoded BTEQZ branch instruction that has a limited span
>and can overflow causing an assembly failure if the target label is too
>distant.  GCC is able to relax out of range MIPS16 branches these days,
>but obviously they have to be expressed in RTL rather than buried in
>assembly code.  This overflow can be easily reproduced; please enquire
>for a boring example if interested.
> 
> 2. The `casesi_internal_mips16_' pattern now has an accurate length
>(aka instruction count) recorded as all its remaining emitted assembly
>instructions are known in advance to fit in their regular (16-bit)
>machine encodings.  Previously there was uncertainty about the SLTU and
>BTEQZ instructions used for the bounds check, which depending on their
>exact arguments could have each resulted in their either regular
>(16-bit) or extended (32-bit) encoding.  Consequently the worst length
>estimate was recorded instead, possibly causing worse code generation
>(e.g. premature out-of-range branch relaxation or jump table expansion
>from half-words to full words).
> 
> 3. GCC now has freedom to schedule code around bounds checking as it sees
>fit rather than having to adapt to the fixed assembly block.  In fact
>it tends to make use of it right away swapping BTEQZ for the BTNEZ
>instruction and rescheduling code such that the out-of-bounds (default)
>case executes linearly.
> 
> There are probably more benefits, but these are what has immediately come
> to my mind.

Sounds good, I certainly agree that we will see the benefits you list above.

>  As noted above I meant to propose this change along with the rest so as
> to have it in GCC 7, however emitting the bounds check as a separate RTL
> pattern triggered an unrelated bug, then unknown to me, causing a fatal
> code generation regression, and the lack of time did not allow me to
> investigate it further.  This was easily reproduced with a piece of code
> (reduced from actual Linux kernel code) like this:
> 
> $ cat frob.c
> int
> frob (int i)
> {
>   switch (i)
> {
> case -5:
>   return -2;
> case -3:
>   return -1;
> case 0:
>   return 0;
> case 3:
>   return 1;
> case 5:
>   break;
> default:
>   __builtin_unreachable ();
> }
>   return i;
> }
> 
> producing truncated assembly like this:
> 
> $ gcc -O2 -mips16 -mcode-readable=yes -dp -S frob.c
> $ cat frob.s
>   .file   1 "frob.c"
>   .section .mdebug.abi32
>   .previous
>   .nanlegacy
>   .module fp=32
>   .module nooddspreg
>   .abicalls
>   .option pic0
>   .text
>   .align  2
>   .weak   frob
>   .setmips16
>   .setnomicromips
>   .entfrob
>   .type   frob, @function
> frob:
>   .frame  $sp,0,$31   # vars= 0, regs= 0/0, args= 0, gp= 0
>   .mask   0x,0
>   .fmask  0x,0
>   addiu   $2,$4,5  # 11   *addsi3_mips16/7[length = 2]
>   .endfrob
>   .size   frob, .-frob
>   .ident  "GCC: (GNU) 7.0.0 20161117 (experimental)"
> $
> 
> -- where as you can see the whole switch statement has vanished along with
> any return path from the function, and only the preparatory addition
> emitted from `casesi' with:
> 
>   emit_insn (gen_addsi3 (reg, operands[0], offset));
> 
> remained.
> 
>  The causing bug has turned out to be what was filed as PR
> tree-optimization/51513 and has been kindly fixed by Peter recently
> (thanks, Peter!) with r247844 ("Fix PR51513, switch statement with default
> case containing __builtin_unreachable leads to wild branch"),
> , enabling me to
> proceed with this change without having to investigate the cause of code
> breakage -- which for the MIPS16 target has clearly turned out to be
> graver than a mere silly branch never to be executed.
> 
>  Given the previous troubles with this change I have decided to add
> MIPS16 test cases to verify that code truncation has not happened,
> complementing gcc.target/powerpc/pr51513.c, in case further tweaks in this
> area might do something bad.  This would be caught by
> gcc.target/mips/insn-casesi.c added with r242424, but that test case does
> not refer to PR tree-optimization/51513, so let's make it explicit.  With
> the PR tree-optimization/51513 fix removed the two new cases indeed cause:
> 
> FAIL: gcc.target/mips/pr51513-1.c   -O2   scan-assembler \tjrc?\t\\$31\n
> FAIL: gcc.target/mips/pr51513-1.c   -O3 -g   scan-assembler

RE: Add support for use_hazard_barrier_return function attribute

[Matthew Fortune]

Prachi Godbole  writes:
> Changelog:
> 
> 2017-04-25  Prachi Godbole  
> 
> gcc/
>   * config/mips/mips.h (machine_function): New variable
>   use_hazard_barrier_return_p.
>   * config/mips/mips.md (UNSPEC_JRHB): New unspec.
>   (mips_hb_return_internal): New insn pattern.
>   * config/mips/mips.c (mips_attribute_table): Add attribute
>   use_hazard_barrier_return.
>   (mips_use_hazard_barrier_return_p): New static function.
>   (mips_function_attr_inlinable_p): Likewise.
>   (mips_compute_frame_info): Set use_hazard_barrier_return_p.  Emit
> error
>   for unsupported architecture choice.
>   (mips_function_ok_for_sibcall, mips_can_use_return_insn): Return
> false
>   for use_hazard_barrier_return.
>   (mips_expand_epilogue): Emit hazard barrier return.
>   * doc/extend.texi: Document use_hazard_barrier_return.
> 
> gcc/testsuite/
>   * gcc.target/mips/hazard-barrier-return-attribute.c: New test.

Sorry for not getting back to this after stage1 opened.

This looks like a great idea.  I'm slightly concerned about what will
happen if code uses this attribute and is built by older tools.  The
problem being that it will just get a warning and that may or may not
be strong enough for a user to notice they did not get a jr.hb and
then go on to get weird runtime failures.

That said we don't have this kind of feature detection for all the new
attributes relating to interrupts so perhaps we just leave this to
-Werror and/or configure time detection of the feature.

No major issues with this just a little cleanup...

> Index: gcc/doc/extend.texi
> ===
> --- gcc/doc/extend.texi   (revision 246899)
> +++ gcc/doc/extend.texi   (working copy)
> @@ -4496,6 +4496,12 @@ On MIPS targets, you can use the
> @code{nocompressi
>  to locally turn off MIPS16 and microMIPS code generation.  This attribute
>  overrides the @option{-mips16} and @option{-mmicromips} options on the
>  command line (@pxref{MIPS Options}).
> +
> +@item use_hazard_barrier_return
> +@cindex @code{use_hazard_barrier_return} function attribute, MIPS
> +This function attribute instructs the compiler to generate hazard barrier 
> return

Missing an 'a':

... generate a hazard barrier return...

Documentation normally wraps at 74 columns which makes these lines a
bit long.

> +that clears all execution and instruction hazards while returning, instead of
> +generating a normal return instruction.
>  @end table
> 
>  @node MSP430 Function Attributes
> Index: gcc/config/mips/mips.md
> ===
> --- gcc/config/mips/mips.md   (revision 246899)
> +++ gcc/config/mips/mips.md   (working copy)
> @@ -156,6 +156,7 @@
> 
>;; The `.insn' pseudo-op.
>UNSPEC_INSN_PSEUDO
> +  UNSPEC_JRHB

Add a comment on what the unspec is.

>  ])
> 
>  (define_constants
> @@ -6578,6 +6579,20 @@
>[(set_attr "type"  "jump")
> (set_attr "mode"  "none")])
> 
> +;; Insn to clear execution and instruction hazards while returning.
> +;; However, it doesn't clear hazards created by the insn in its delay slot.
> +;; Thus, explicitly place a nop in its delay slot.
> +
> +(define_insn "mips_hb_return_internal"
> +  [(return)
> +   (unspec_volatile [(match_operand 0 "pmode_register_operand" "")]
> + UNSPEC_JRHB)]
> +  ""
> +  {
> +return "%(jr.hb\t$31%/%)";
> +  }
> +  [(set_attr "insn_count" "2")])
> +
>  ;; Normal return.
> 
>  (define_insn "_internal"
> Index: gcc/config/mips/mips.c
> ===
> --- gcc/config/mips/mips.c(revision 246899)
> +++ gcc/config/mips/mips.c(working copy)
> @@ -615,6 +615,7 @@ static const struct attribute_spec mips_attribute_
>  mips_handle_use_shadow_register_set_attr, false },
>{ "keep_interrupts_masked",0, 0, false, true,  true, NULL, false },
>{ "use_debug_exception_return", 0, 0, false, true,  true, NULL, false },
> +  { "use_hazard_barrier_return", 0, 0, true, false, false, NULL, false },
>{ NULL,   0, 0, false, false, false, NULL, false }
>  };
> 
> 
> @@ -1275,6 +1276,16 @@ mips_use_debug_exception_return_p (tree type)
>  TYPE_ATTRIBUTES (type)) != NULL;
>  }
> 
> +/* Check if the attribute to use hazard barrier return is set for
> +   the function declaration DECL.  */
> +
> +static bool
> +mips_use_hazard_barrier_return_p (tree decl)

Make this a const_tree so you don't have to const_cast.

> +{
> +  return lookup_attribute ("use_hazard_barrier_return",
> + DECL_ATTRIBUTES (decl)) != NULL;
> +}
> +

DECL_ATTRIBUTES is indented 1 space too many.

>  /* Return the set of compression modes that are explicitly required
> by the attributes in ATTRIBUTES.  */
> 
> @@ -1460,6 +1471,21 @@ mips_can_inline_p (tree caller, tree callee)
>return default_target_can_inline_p (caller, callee);
>  }
> 
> +/

[PATCH,committed] [MAINTAINERS] Update email address

[Matthew Fortune]

Updating my email address, apologies for being out of date for a while.

Matthew

* MAINTAINERS: Update my email address.

---
 ChangeLog   | 4 
 MAINTAINERS | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f9f376a..54b7958 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -77,7 +77,7 @@ m68k port Andreas Schwab

 m68k-motorola-sysv portPhilippe De Muyter  
 mcore port Nick Clifton
 microblaze Michael Eager   
-mips port  Matthew Fortune 
+mips port  Matthew Fortune 
 mmix port  Hans-Peter Nilsson  
 mn10300 port   Jeff Law
 mn10300 port   Alexandre Oliva 
-- 
2.2.1

RE: Prefer open-coding vector integer division

[Matthew Fortune]

Richard Sandiford  writes:
> vect_recog_divmod_pattern currently bails out if the target has
> native support for integer division, but I think in practice
> it's always going to be better to open-code it anyway, just as
> we usually open-code scalar divisions by constants.
> 
> I think the only currently affected target is MIPS MSA, where for:
> 
>   void
>   foo (int *x)
>   {
> for (int i = 0; i < 100; ++i)
>   x[i] /= 2;
>   }
> 
> we previously preferred to use division for powers of 2:
> 
> .setnoreorder
> bnz.w   $w1,1f
> div_s.w $w0,$w0,$w1
> break   7
> .setreorder
> 1:
> 
> (or just the div_s.w for -mno-check-zero-division), but after the patch
> we open-code them using shifts:
> 
> clt_s.w $w1,$w0,$w2
> subv.w  $w0,$w0,$w1
> srai.w  $w0,$w0,1
> 
> I assume that's better.  Matthew, is that right?

Sorry for extreme tardiness. Yes, the alternate sequence has a max latency
of 6. Although I don't have the range of latencies to hand for the FDIV, as
far as I remember 6 cycles is better than the fastest FDIV case at least for
i6400/i6500.

Matthew

RE: [PATCH] MIPS: Add support for -mcrc and -mginv options

[Matthew Fortune]

Robert Suchanek  writes:
> This patch adds -mcrc and -mginv options to pass through them
> to the assembler.
> 
> Regards,
> Robert
> 
> gcc/ChangeLog:
> 
> 2018-06-01  Matthew Fortune  
> 
>   * config/mips/mips.h (ASM_SPEC): Pass through -mcrc, -mno-crc,
>   -mginv and -mno-ginv to the assembler.
>   * config/mips/mips.opt (-mcrc): New option.
>   (-mginv): Likewise.
>   * doc/invoke.text (-mcrc): Document.
>   (-mginv): Likewise.

The patch is OK but should probably wait until the binutils support is
committed. I see CRC ASE support in discussion up until:

https://sourceware.org/ml/binutils/2017-12/msg00069.html

And GINV support up to:

https://sourceware.org/ml/binutils/2018-01/msg00125.html

Thanks,
Matthew

RE: [PATCH] MIPS: Update I6400 scheduler

[Matthew Fortune]

Robert Suchanek  writes:
> Update to i6400 scheduler.
> 
> Regards,
> Robert
> 
> gcc/ChangeLog:
> 
> 2018-06-01  Prachi Godbole  
> 
>   * config/mips/i6400.md (i6400_gpmuldiv): Remove cpu_unit.
>   (i6400_gpmul): Add cpu_unit.
>   (i6400_gpdiv): Likewise.
>   (i6400_msa_add_d): Update reservations.
>   (i6400_msa_int_add) Likewise.
>   (i6400_msa_short_logic3) Likewise.
>   (i6400_msa_short_logic2) Likewise.
>   (i6400_msa_short_logic) Likewise.
>   (i6400_msa_move) Likewise.
>   (i6400_msa_cmp) Likewise.
>   (i6400_msa_short_float2) Likewise.
>   (i6400_msa_div_d) Likewise.
>   (i6400_msa_long_logic1) Likewise.
>   (i6400_msa_long_logic2) Likewise.
>   (i6400_msa_mult) Likewise.
>   (i6400_msa_long_float2) Likewise.
>   (i6400_msa_long_float4) Likewise.
>   (i6400_msa_long_float5) Likewise.
>   (i6400_msa_long_float8) Likewise.
>   (i6400_fpu_minmax): New define_insn_reservation.
>   (i6400_fpu_fadd): Include frint type.
>   (i6400_fpu_store): New define_insn_reservation.
>   (i6400_fpu_load): Likewise.
>   (i6400_fpu_move): Likewise.
>   (i6400_fpu_fcmp): Likewise.
>   (i6400_fpu_fmadd): Likewise.
>   (i6400_int_mult): Include imul3nc type and update reservation.
>   (i6400_int_div): Include idiv3 type and update reservation.
>   (i6400_int_load): Update to check type not move_type.
>   (i6400_int_store): Likewise.
>   (i6400_int_prefetch): Set zero latency.

Hi Robert,

Just to fill in the blanks on the submission history for this. The
patch here was written by Prachi while MIPS was a part of IMG and
was then transferred to MIPS Tech LLC when the business split. Both
IMG and MIPS Tech LLC have copyright assignment in place so it
does not matter that it is authored by someone from IMG and submitted
by a MIPS Tech LLC employee.

There is no public platform available to demonstrate the improvement
of this patch but it was tested, when written, to meet or exceed
existing performance on i6400. It is also included in the reference
tools for the i6400 produced by MIPS Tech LLC.

There does seem to be a temporal issue in submission for this as
the i6400_fpu_minmax reservation refers to fminmax and fclass types
that do not exist in trunk. Can you drop that reservation please?

Otherwise, OK to commit.

Thanks,
Matthew

> ---
>  gcc/config/mips/i6400.md | 86 ++--
> 
>  1 file changed, 61 insertions(+), 25 deletions(-)
> 
> diff --git a/gcc/config/mips/i6400.md b/gcc/config/mips/i6400.md
> index 413e9e8..a985401 100644
> --- a/gcc/config/mips/i6400.md
> +++ b/gcc/config/mips/i6400.md
> @@ -21,7 +21,7 @@
>  (define_automaton "i6400_int_pipe, i6400_mdu_pipe,
> i6400_fpu_short_pipe,
>  i6400_fpu_long_pipe")
> 
> -(define_cpu_unit "i6400_gpmuldiv" "i6400_mdu_pipe")
> +(define_cpu_unit "i6400_gpmul, i6400_gpdiv" "i6400_mdu_pipe")
>  (define_cpu_unit "i6400_agen, i6400_alu1, i6400_lsu" "i6400_int_pipe")
>  (define_cpu_unit "i6400_control, i6400_ctu, i6400_alu0"
> "i6400_int_pipe")
> 
> @@ -50,49 +50,49 @@ (define_insn_reservation "i6400_msa_add_d" 1
>(and (eq_attr "cpu" "i6400")
> (and (eq_attr "mode" "!V2DI")
>   (eq_attr "alu_type" "simd_add")))
> -  "i6400_fpu_short, i6400_fpu_intadd")
> +  "i6400_fpu_short+i6400_fpu_intadd*2")
> 
>  ;; add, hadd, sub, hsub, average, min, max, compare
>  (define_insn_reservation "i6400_msa_int_add" 2
>(and (eq_attr "cpu" "i6400")
> (eq_attr "type" "simd_int_arith"))
> -  "i6400_fpu_short, i6400_fpu_intadd")
> +  "i6400_fpu_short+i6400_fpu_intadd*2")
> 
>  ;; sat, pcnt
>  (define_insn_reservation "i6400_msa_short_logic3" 3
>(and (eq_attr "cpu" "i6400")
> (eq_attr "type" "simd_sat,simd_pcnt"))
> -  "i6400_fpu_short, i6400_fpu_logic")
> +  "i6400_fpu_short+i6400_fpu_logic*2")
> 
>  ;; shifts, nloc, nlzc, bneg, bclr, shf
>  (define_insn_reservation "i6400_msa_short_logic2" 2
>(and (eq_attr "cpu" "i6400")
> (eq_attr "type" "simd_shift,simd_shf,simd_bit"))
> -  "i6400_fpu_short, i6400_fpu_logic")
> +  "i6400_fpu_short+i6400_fpu_logic*2")
> 
>  ;; and, or, xor, ilv, pck, fill, splat
>  (define_insn_reservation "i6400_msa_short_logic" 1
>(and (eq_attr "cpu" "i6400")
> (eq_attr "type"
> "simd_permute,simd_logic,simd_splat,simd_fill"))
> -  "i6400_fpu_short, i6400_fpu_logic")
> +  "i6400_fpu_short+i6400_fpu_logic*2")
> 
>  ;; move.v, ldi
>  (define_insn_reservation "i6400_msa_move" 1
>(and (eq_attr "cpu" "i6400")
> (eq_attr "type" "simd_move"))
> -  "i6400_fpu_short, i6400_fpu_logic")
> +  "i6400_fpu_short+i6400_fpu_logic*2")
> 
>  ;; Float compare New: CMP.cond.fmt
>  (define_insn_reservation "i6400_msa_cmp" 2
>(and (eq_attr "cpu" "i6400")
> (eq_attr "type" "simd_fcmp"))
> -  "i6400_fpu_short, i6400_fpu_cmp")
> +  "i6400_fpu_short+i6400_fpu_cmp*2")
> 
>  ;; Float min, max, class
>  (define_insn_reservation "i6400_msa_sh

RE: [PATCH] MIPS: Add i6500 processor as an alias for i6400

[Matthew Fortune]

Robert Suchanek  writes:
> This patch adds i6500 CPU as an alias for i6400.
> 
> Regards,
> Robert
> 
> gcc/ChangeLog:
> 
> 2018-06-01  Matthew Fortune  
> 
>   * config/mips/mips-cpus.def: New MIPS_CPU for i6500.
>   * config/mips/mips-tables.opt: Regenerate.
>   * config/mips/mips.h (MIPS_ISA_LEVEL_SPEC): Mark i6500 as
>   mips64r6.
>   * doc/invoke.texi: Document -march=i6500.

Looks good, OK to commit.

Thanks,
Matthew

RE: [PATCH] MIPS: Add support for P6600

[Matthew Fortune]

Robert Suchanek  writes:
> The below adds support for -march=p6600.  It includes
> a new scheduler plus performance tweaks.
> 
> gcc/ChangeLog:
> 
> 2018-06-01  Matthew Fortune  
> Prachi Godbole  
>   * config/mips/mips-cpus.def: Define P6600.
>   * config/mips/mips-tables.opt: Regenerate.
>   * config/mips/mips.c (mips_multipass_dfa_lookahead): Add P6600.
>   * config/mips/mips.h (TUNE_P6600): New define.
>   (MIPS_ISA_LEVEL_SPEC): Infer mips64r6 from p6600.
>   * doc/invoke.texi: Add p6600 to supported architectures.

It seems that mips.com has no longer got any architecture specific
documents available for either i6400/i6500 (which did exist until
recently) nor p6600. There isn't anything particularly unusual
about i6400/i6500 support but this patch for p6600 attempts to deal
with some undocumented micro-architecture details that are not
sufficiently described in the patch to maintain over time nor is
there reference material available.

Despite the fact that I am one of the original authors of the
work, I can't accept this upstream without further information.

Some notes on the patch are below:

> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index bfe64bb..9c77c13 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -198,6 +198,15 @@ enum mips_address_type {
>ADDRESS_SYMBOLIC
>  };
> 
> +/* Classifies an unconditional branch of interest for the P6600.  */
> +
> +enum mips_ucbranch_type {

Newline for brace.

> +  /* May not even be a branch.  */
> +  UC_UNDEFINED,
> +  UC_BALC,
> +  UC_OTHER
> +};
> +
>  /* Macros to create an enumeration identifier for a function
> prototype.  */
>  #define MIPS_FTYPE_NAME1(A, B) MIPS_##A##_FTYPE_##B
>  #define MIPS_FTYPE_NAME2(A, B, C) MIPS_##A##_FTYPE_##B##_##C
> @@ -1127,6 +1136,19 @@ static const struct mips_rtx_cost_data
>  COSTS_N_INSNS (36),   /* int_div_di */
>   2,/* branch_cost */
>   4 /* memory_latency */
> +  },
> +  { /* P6600 */
> +COSTS_N_INSNS (4),/* fp_add */
> +COSTS_N_INSNS (5),/* fp_mult_sf */
> +COSTS_N_INSNS (5),/* fp_mult_df */
> +COSTS_N_INSNS (17),   /* fp_div_sf */
> +COSTS_N_INSNS (17),   /* fp_div_df */
> +COSTS_N_INSNS (5),/* int_mult_si */
> +COSTS_N_INSNS (5),/* int_mult_di */
> +COSTS_N_INSNS (8),/* int_div_si */
> +COSTS_N_INSNS (8),/* int_div_di */
> + 2,/* branch_cost */
> + 4 /* memory_latency */
>}
>  };
> 
> 
> @@ -14592,6 +14614,7 @@ mips_issue_rate (void)
>  case PROCESSOR_LOONGSON_2F:
>  case PROCESSOR_LOONGSON_3A:
>  case PROCESSOR_P5600:
> +case PROCESSOR_P6600:
>return 4;
> 
>  case PROCESSOR_XLP:
> @@ -14727,7 +14750,7 @@ mips_multipass_dfa_lookahead (void)
>if (TUNE_OCTEON)
>  return 2;
> 
> -  if (TUNE_P5600 || TUNE_I6400)
> +  if (TUNE_P5600 || TUNE_P6600 || TUNE_I6400)
>  return 4;
> 
>return 0;
> @@ -18647,6 +18670,28 @@ mips_orphaned_high_part_p (mips_offset_table
> *htab, rtx_insn *insn)
>return false;
>  }
> 
> +/* Subroutine of mips_avoid_hazard.  We classify unconditional
> branches
> +   of interest for the P6600 for performance reasons.  We're
> interested
> +   in differentiating BALC from JIC, JIALC and BC.  */
> +
> +static enum mips_ucbranch_type
> +mips_classify_branch_p6600 (rtx_insn *insn)
> +{
> +  if (!(insn
> +  && USEFUL_INSN_P (insn)
> +  && GET_CODE (PATTERN (insn)) != SEQUENCE))
> +return UC_UNDEFINED;

Let's switch this around to the following with a comment to say
ignore sequences because they represent a filled delay slot
branch (which presumably is not affected by the uArch issue).

  if (insn
  || !USEFUL_INSN_P (insn)
  || GET_CODE (PATTERN (insn)) == SEQUENCE))
return UC_UNDEFINED;

> +
> +  if (get_attr_jal (insn) == JAL_INDIRECT /* JIC and JIALC.  */
> +  || get_attr_type (insn) == TYPE_JUMP) /* BC as it is a loose
> jump.  */
> +return UC_OTHER;

I don't recall what a loose jump was supposed to refer to, presumably
'direct jump'. 

>  /* Subroutine of mips_reorg_process_insns.  If there is a hazard
> between
> INSN and a previous instruction, avoid it by inserting nops after
> instruction AFTER.
> @@ -18699,14 +18744,36 @@ mips_avoid_hazard (rtx_insn *after, rtx_insn
> *insn, int *hilo_delay,
>  && GET_CODE (pattern) != ASM_INPUT
>  && asm_noperands (pattern) < 0)
>   

RE: [PATCH,MIPS] Fix pr86067 ICE: scal-to-vec1.c:86:1: error: unrecognizable insn with -march=loongson3a

[Matthew Fortune]

Paul Hua  writes:
> The gcc.c-torture/execute/scal-to-vec1.c  trigger a gcc ICE bug.
> 
> It's a mistake in define_expand vec_setv4hi in loongson.md file.
> 
> 375 (define_expand "vec_setv4hi"
> 376   [(set (match_operand:V4HI 0 "register_operand" "=f")
> 377 (unspec:V4HI [(match_operand:V4HI 1 "register_operand" "f")
> 378   (match_operand:HI 2 "register_operand" "f")
> 379   (match_operand:SI 3 "const_0_to_3_operand"
> "")]
> 380  UNSPEC_LOONGSON_PINSRH))]
> 381   "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
> 382 {
> 383   rtx ext = gen_reg_rtx (SImode);
> 384   emit_move_insn (ext, gen_lowpart (SImode, operands[1]));
> 385   operands[1] = ext;
> 386 })
> 
> The line 384 gen_lowpart the operands[1], should be gen_lowpart
> operands[2], cause the operands[2] are HImode.
> 
> 
> The attached patch fixed this bug.
> 
> Bootstrapped and reg-tested on mips64el-unknown-linux-gnu.
> Ok for commit ?
> 
> 
> ---

Hi Paul,

This looks good, just one issue with the changelog entry. The entry
would go in the gcc/ChangeLog file and the path is then relative to
the gcc/ directory. The PR should be referenced as target/:

2018-03-24  Chenghua Xu 
 
PR target/86076
* config/mips/loongson.md (vec_setv4hi): Gen_lowpart for
operands[2] instead of operands[1].

OK to commit, thanks for finding and fixing. This has been broken
since 2011!

Matthew

[RFC] New features for multilib control

[Matthew Fortune]

Hi,

This patch was developed as part of preparing ever more complex multilib
combinations for the MIPS architecture and aims to solve several problems
in this area. I've attempted to be quite verbose in the description, so
that I can explain how I am using various terms as pretty much everyone
has a different understanding (and I don't claim mine to be 'right'
either).

The changes aim to:

1) Eliminate the fallback multilib
The fallback multilib (top level of 'lib') is often annoying because it is
used for any combination of options that do not match a specific multilib.
Quite often this default multilib is incompatible with the build options
that end up linking against it, leading to bizarre link time messages that
confuse ordinary users.

2) Move the default multilib to a subfolder
Having successfully eliminated the fallback multilib it is also true that
it would eliminate the 'default' multilib as well. I.e. the library used
when no relevant user options are supplied. Moving this library to a
subfolder has two benefits. a) it still exists! and b) the location of
this library becomes invariant irrespective of which options are
build-time configured as default.

3) Preserve/use invariant paths for multilib variants
A simplistic multilib specification leads to a nested set of folders,
where the top level is the left-most multilib directory and the bottom is
the right most. Introducing a new axis of multilib configuration changes
this path structure leading to the different library configurations to 
move around. This is not in itself a problem, as the compiler driver can
always locate the right path for any given build, but it does cause issues
when doing configuration management of binary toolchains. When there are
many different multilibs it is ideal to ship/install only the ones that
are important and if the paths keep changing over time this process is
complex and confusing. This issue is effectively solved by the
MULTI_OSDIRNAMES feature but using it for both sysroot and compiler
internal libraries is even better.

4) Support un-released multilib configurations
This one sounds weird but it is quite valuable. When an architecture has
70+ possible multilib variants but only 40 of them are currently known to
be needed then only build and release 40 variants. However, if it turns
out that another configuration becomes useful then it is often handy to
just build the missing configuration and install it into the pre-existing
release. So, the driver needs to know about all multilibs and their paths
but the build process should only build a subset.

So, the solution...

Firstly, be verbose about the MULTILIB_OPTIONS needed for a target. Add
in the default options as well as all the others that may, or could, ever
be supported by the current compiler features. For MIPS supporting
MIPS32r1 onwards it looks like this:

MULTILIB_OPTIONS = muclibc mips32/mips32r2/mips32r6/mips64/mips64r2/mips64r6
mips16/mmicromips mabi=32/mabi=n32/mabi=64 EB/EL msoft-float mnan=2008

This does create an enormous matrix of possible configurations so this
has to be trimmed to the valid set using MULTILIB_REQUIRED. Note that
the valid set should include any that you may wish to support even if
you don't want to build/release them. By having the default options in
then this leads to having two copies of the 'default' multilib; one with
the options implicitly set and one with them explicitly set.

Second, remove the multilib with the implicit default options. This
equates to the '.' multilib entry. To do this use MULTILIB_EXCLUSIONS to
remove the negated form of every MULTILIB_OPTION:

MULTILIB_EXCLUSIONS =
!muclibc/!mips32/!mips32r2/!mips32r6/!mips64/!mips64r2/!mips64r6/!mips16/!mm
icromips/!mabi=32/!mabi=n32/!mabi=64/!EB/!EL/!msoft-float/!mnan=2008

Third, set the MULTILIB_OSDIRNAMES to have an entry for every valid
combination of options and use the '!' modifier. Honestly, I'm not sure
how/why this works but this leads to both the internal library paths and
sysroot paths using the OSDIRNAME instead of the nested tree of
MULTILIB_DIRNAMES. Choose a path for each variant that you will never
change again; I used an encoded form of the configuration for MIPS:

MULTILIB_OSDIRNAMES =
mips32r6/mabi.32/EB/mnan.2008=!mips-r6-hard$(is_newlib)/lib

Fourth, deal with the fallout from the config-ml.in logic which handles
the initial 'configure' of a library differently to all of the multilib
configurations. The basic idea is that since the default multilib will now
report that it belongs in a subdirectory then, when configuring the top
level multilib, query the driver for the folder. Later when iterating
through the multilib specs skip the one that matches the default
configuration. Each configuration will be built exactly once and all of
them will be placed in a subfolder leaving the top level install folder
bare.

Fifth, restrict the set of multilibs that actually get built for any
given compiler. This is sort-of a new concept so I added a
-

RE: [PATCH] MIPS: Add support for P6600

[Matthew Fortune]

Robert Suchanek  writes:
> As already discussed, the link to the P6600 doesn't
> appear to be referenced on mips.com but reachable
> when searching for 'p6600':
> 
> https://www.mips.com/downloads/p6600-multiprocessing-programmers-guide/

Thanks, good spot.

> gcc/ChangeLog:
> 
> 2018-06-12  Matthew Fortune  
> Prachi Godbole  
> 
>   * config/mips/mips-cpus.def: Define P6600.
>   * config/mips/mips-tables.opt: Regenerate.
>   * config/mips/mips.c (mips_ucbranch_type): New enum.
>   (mips_rtx_cost_data): Add support for P6600.
>   (mips_issue_rate): Likewise.
>   (mips_multipass_dfa_lookahead): Likewise.
>   (mips_avoid_hazard): Likewise.
>   (mips_reorg_process_insns): Likewise.
>   (mips_classify_branch_p6600): New function.
>   * config/mips/mips.h (TUNE_P6600): New define.
>   (MIPS_ISA_LEVEL_SPEC): Infer mips64r6 from p6600.
>   (ENABLE_LD_ST_PAIRS): Enable load/store bonding for p6600.
>   * config/mips/mips.md: Include p6600.md.
>   (processor): Add p6600.
>   * config/mips/p6600.md: New file.
>   * doc/invoke.texi: Add p6600 to supported architectures.

With one more change to add another comment as below, this is OK to
commit.

> @@ -18957,7 +19039,10 @@ mips_reorg_process_insns (void)
>sequence and replace it with the delay slot instruction
>then the jump to clear the forbidden slot hazard.  */

This bit does need the comment extending.  Add this:

For the P6600, this optimisation solves the performance penalty
associated with BALC followed by a delay slot branch.  We do not set
fs_delay as we do not want the full logic of a forbidden slot; the
penalty exists only against branches not the full class of forbidden
slot instructions.

> 
> -   if (fs_delay)
> +   if (fs_delay || (TUNE_P6600
> +&& TARGET_CB_MAYBE
> +&& mips_classify_branch_p6600 (insn)
> +   == UC_BALC))
>   {
> /* Search onwards from the current position looking
> for
>a SEQUENCE.  We are looking for pipeline hazards
here

Thanks,
Matthew

RE: [PATCH] MIPS: Add support for -mcrc and -mginv options

[Matthew Fortune]

Robert Suchanek  writes:
> This patch adds -mcrc and -mginv options to pass through them
> to the assembler.
> 
> Regards,
> Robert
> 
> gcc/ChangeLog:
> 
> 2018-06-01  Matthew Fortune  
> 
>   * config/mips/mips.h (ASM_SPEC): Pass through -mcrc, -mno-crc,
>   -mginv and -mno-ginv to the assembler.
>   * config/mips/mips.opt (-mcrc): New option.
>   (-mginv): Likewise.
>   * doc/invoke.text (-mcrc): Document.
>   (-mginv): Likewise.

Since CRC and GINV ASEs have now been committed to binutils, please
go ahead with this change.

Thanks,
Matthew

RE: [PATCH, PR83327] Fix liveness analysis in lra for spilled-into hard regs

[Matthew Fortune]

Tom de Vries  writes:
> On 01/08/2018 05:32 PM, Tom de Vries wrote:
> > On 12/18/2017 05:57 PM, Vladimir Makarov wrote:
> >>
> >>
> >> On 12/15/2017 06:25 AM, Tom de Vries wrote:
> >>>
> >>> Proposed Solution:
> >>>
> >>> The patch addresses the problem, by:
> >>> - marking the hard regs that have been used in lra_spill in
> >>>   hard_regs_spilled_into
> >>> - using hard_regs_spilled_into in lra_create_live_ranges to
> >>>   make sure those registers are marked in the conflict_hard_regs
> >>>   of pseudos that overlap with the spill register usage
> >>>
> >>> [ I've also tried an approach where I didn't use
> >>> hard_regs_spilled_into, but tried to propagate all hard regs. I
> >>> figured out that I needed to mask out eliminable_regset.  Also I
> >>> needed to masked out lra_no_alloc_regs, but that could be due to
> >>> gcn-specific problems (pointers take 2 hard regs), I'm not yet sure.
> >>> Anyway, in the submitted patch I tried to avoid these problems and
> >>> went for the more minimal approach. ]
> >>>
> >> Tom, thank you for the detail explanation of the problem and
> >> solutions you considered.  It helped me a lot.  Your simple solution
> >> is adequate as the most transformations and allocation are done on
> >> the 1st LRA subpasses iteration.
> >>> In order to get the patch accepted for trunk, I think we need:
> >>> - bootstrap and reg-test on x86_64
> >>> - build and reg-test on mips (the only primary platform that has the
> >>>   spill_class hook enabled)
> >>>
> >>> Any comments?
> >>
> >> The patch looks ok to me.  You can commit it after successful testing
> >> on x86-64 and mips but I am sure there will be no problems with
> >> x86-64 as it does not use spill_class currently (actually your patch
> >> might help to switch it on again for x86-64.  spill_class was quite
> >> useful for x86-64 performance on Intel processors).
> >>
> >
> > Hi Matthew,
> >
> > there's an lra optimization that is currently enabled for MIPS, and
> > not for any other primary or secondary target.
> >
> > This (already approved) patch fixes a bug in that optimization, and
> > needs to be tested on MIPS.
> >
> > Unfortunately, the optimization is only enabled for MIPS16, and we
> > don't have a current setup to test this.
> >
> > Could you help us out here and test this patch for MIPS16 on trunk?
> 
> Hi Matthew,
> 
> is this something you can help us out with?

Hi Tom,

I've just commented on the bug report that I've set of some builds to try
and give some assurance. It is far from comprehensive but it is as good as
the normal testing I do for MIPS16.

Thanks,
Matthew

RE: [PATCH, PR83327] Fix liveness analysis in lra for spilled-into hard regs

[Matthew Fortune]

Tom de Vries  writes:
> On 02/26/2018 12:00 PM, Matthew Fortune wrote:
> > Tom de Vries  writes:
> >> On 01/08/2018 05:32 PM, Tom de Vries wrote:
> >>> On 12/18/2017 05:57 PM, Vladimir Makarov wrote:
> >>>>
> >>>>
> >>>> On 12/15/2017 06:25 AM, Tom de Vries wrote:
> >>>>>
> >>>>> Proposed Solution:
> >>>>>
> >>>>> The patch addresses the problem, by:
> >>>>> - marking the hard regs that have been used in lra_spill in
> >>>>>    hard_regs_spilled_into
> >>>>> - using hard_regs_spilled_into in lra_create_live_ranges to
> >>>>>    make sure those registers are marked in the conflict_hard_regs
> >>>>>    of pseudos that overlap with the spill register usage
> >>>>>
> >>>>> [ I've also tried an approach where I didn't use
> >>>>> hard_regs_spilled_into, but tried to propagate all hard regs. I
> >>>>> figured out that I needed to mask out eliminable_regset.  Also I
> >>>>> needed to masked out lra_no_alloc_regs, but that could be due to
> >>>>> gcn-specific problems (pointers take 2 hard regs), I'm not yet
> sure.
> >>>>> Anyway, in the submitted patch I tried to avoid these problems and
> >>>>> went for the more minimal approach. ]
> >>>>>
> >>>> Tom, thank you for the detail explanation of the problem and
> >>>> solutions you considered.  It helped me a lot.  Your simple
> >>>> solution is adequate as the most transformations and allocation are
> >>>> done on the 1st LRA subpasses iteration.
> >>>>> In order to get the patch accepted for trunk, I think we need:
> >>>>> - bootstrap and reg-test on x86_64
> >>>>> - build and reg-test on mips (the only primary platform that has
> >>>>> the
> >>>>>    spill_class hook enabled)
> >>>>>
> >>>>> Any comments?
> >>>>
> >>>> The patch looks ok to me.  You can commit it after successful
> >>>> testing on x86-64 and mips but I am sure there will be no problems
> >>>> with
> >>>> x86-64 as it does not use spill_class currently (actually your
> >>>> patch might help to switch it on again for x86-64.  spill_class was
> >>>> quite useful for x86-64 performance on Intel processors).
> >>>>
> >>>
> >>> Hi Matthew,
> >>>
> >>> there's an lra optimization that is currently enabled for MIPS, and
> >>> not for any other primary or secondary target.
> >>>
> >>> This (already approved) patch fixes a bug in that optimization, and
> >>> needs to be tested on MIPS.
> >>>
> >>> Unfortunately, the optimization is only enabled for MIPS16, and we
> >>> don't have a current setup to test this.
> >>>
> >>> Could you help us out here and test this patch for MIPS16 on trunk?
> >>
> >> Hi Matthew,
> >>
> >> is this something you can help us out with?
> >
> > Hi Tom,
> >
> > I've just commented on the bug report that I've set of some builds to
> > try and give some assurance. It is far from comprehensive but it is as
> > good as the normal testing I do for MIPS16.
> >
> 
> Hi Matthew,
> 
> Awesome, thanks for the help.

I'm afraid my lack of attention has shown that there appears to be a
bug preventing libstdc++ building with MIPS16 that was introduced
somewhere between Oct 9th and Oct 10th. 47 commits left to bisect.

Matthew

RE: [PATCH, PR83327] Fix liveness analysis in lra for spilled-into hard regs

[Matthew Fortune]

Tom de Vries  writes:
> On 02/26/2018 12:00 PM, Matthew Fortune wrote:
> > Tom de Vries  writes:
> >> On 01/08/2018 05:32 PM, Tom de Vries wrote:
> >>> On 12/18/2017 05:57 PM, Vladimir Makarov wrote:
> >>>>
> >>>>
> >>>> On 12/15/2017 06:25 AM, Tom de Vries wrote:
> >>>>>
> >>>>> Proposed Solution:
> >>>>>
> >>>>> The patch addresses the problem, by:
> >>>>> - marking the hard regs that have been used in lra_spill in
> >>>>>    hard_regs_spilled_into
> >>>>> - using hard_regs_spilled_into in lra_create_live_ranges to
> >>>>>    make sure those registers are marked in the conflict_hard_regs
> >>>>>    of pseudos that overlap with the spill register usage
> >>>>>
> >>>>> [ I've also tried an approach where I didn't use
> >>>>> hard_regs_spilled_into, but tried to propagate all hard regs. I
> >>>>> figured out that I needed to mask out eliminable_regset.  Also I
> >>>>> needed to masked out lra_no_alloc_regs, but that could be due to
> >>>>> gcn-specific problems (pointers take 2 hard regs), I'm not yet
> sure.
> >>>>> Anyway, in the submitted patch I tried to avoid these problems
> and
> >>>>> went for the more minimal approach. ]
> >>>>>
> >>>> Tom, thank you for the detail explanation of the problem and
> >>>> solutions you considered.  It helped me a lot.  Your simple
> solution
> >>>> is adequate as the most transformations and allocation are done on
> >>>> the 1st LRA subpasses iteration.
> >>>>> In order to get the patch accepted for trunk, I think we need:
> >>>>> - bootstrap and reg-test on x86_64
> >>>>> - build and reg-test on mips (the only primary platform that has
> the
> >>>>>    spill_class hook enabled)
> >>>>>
> >>>>> Any comments?
> >>>>
> >>>> The patch looks ok to me.  You can commit it after successful
> testing
> >>>> on x86-64 and mips but I am sure there will be no problems with
> >>>> x86-64 as it does not use spill_class currently (actually your
> patch
> >>>> might help to switch it on again for x86-64.  spill_class was
> quite
> >>>> useful for x86-64 performance on Intel processors).
> >>>>
> >>>
> >>> Hi Matthew,
> >>>
> >>> there's an lra optimization that is currently enabled for MIPS, and
> >>> not for any other primary or secondary target.
> >>>
> >>> This (already approved) patch fixes a bug in that optimization, and
> >>> needs to be tested on MIPS.
> >>>
> >>> Unfortunately, the optimization is only enabled for MIPS16, and we
> >>> don't have a current setup to test this.
> >>>
> >>> Could you help us out here and test this patch for MIPS16 on trunk?
> >>
> >> Hi Matthew,
> >>
> >> is this something you can help us out with?
> >
> > Hi Tom,
> >
> > I've just commented on the bug report that I've set of some builds to
> try
> > and give some assurance. It is far from comprehensive but it is as
> good as
> > the normal testing I do for MIPS16.
> >
> 
> Hi Matthew,
> 
> Awesome, thanks for the help.

I have tested trunk with and without the patch and can confirm there
is no change in test status for MIPS16 big endian.

I ended up fixing an assert-checking issue in the MIPS backend and
a bug (I think) in the C++ frontend to get to the point of testing so
quite a worthwhile effort all in all.

Thanks,
Matthew

[PATCH,MIPS,committed] Fix wrong use of XINT instead of INTVAL

[Matthew Fortune]

Hi,

This issue was caught with assert checking enabled but is not a
functional bug as XINT(x, 0) happens to overlay INTVAL(x) anyway.

Committed to trunk.

Thanks,
Matthew

gcc/
* config/mips/mips.c (mips_final_prescan_insn): Fix incorrect
XINT with INTVAL.
(mips_final_postscan_insn): Likewise.
---
 gcc/config/mips/mips.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 00cece2..aabd4b1 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -20426,7 +20426,7 @@ mips_final_prescan_insn (rtx_insn *insn, rtx *opvec,
int noperands)
   && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
   && XINT (PATTERN (insn), 1) == UNSPEC_CONSTTABLE)
 mips_set_text_contents_type (asm_out_file, "__pool_",
-XINT (XVECEXP (PATTERN (insn), 0, 0), 0),
+INTVAL (XVECEXP (PATTERN (insn), 0, 0)),
 FALSE);
 
   if (mips_need_noat_wrapper_p (insn, opvec, noperands))
@@ -20450,7 +20450,7 @@ mips_final_postscan_insn (FILE *file
ATTRIBUTE_UNUSED, rtx_insn *insn,
   && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
   && XINT (PATTERN (insn), 1) == UNSPEC_CONSTTABLE_END)
 mips_set_text_contents_type (asm_out_file, "__pend_",
-XINT (XVECEXP (PATTERN (insn), 0, 0), 0),
+INTVAL (XVECEXP (PATTERN (insn), 0, 0)),
 TRUE);
 }
 
-- 
2.2.1

[PATCH] Fix ICE caused by a missing check for DECL_LANG_SPECIFIC

[Matthew Fortune]

Hi,

It seems we have had a bug for some time that causes an ICE and prevents the
MIPS16 library builds from completing but is likely unrelated to MIPS16.
The problem is when we call target_reinit and library functions get created
as shown in the call stack at the end of this message. The first builtin
that triggers the problem happens to be one of the MIPS16 helpers but I
don't think there is anything unique about it. The issue appeared after some
refactoring work in r253600 where code testing DECL_CLASS_SCOPE_P and
DECL_FRIEND_P was previously guarded by a check for DECL_LANG_SPECIFIC but
not after.

https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00604.html

I don’t know if this is the correct solution or whether we need to change the
way builtins are initialised in the MIPS backend but I suspect this fix
is the right way to go.

Cc: Jason as author of the original change.

Thanks,
Matthew

gcc/cp/
* pt.c (type_dependent_expression_p): Add missing check for
DECL_LANG_SPECIFIC.
---
 gcc/cp/pt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 7345119..c88304f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -24635,6 +24635,7 @@ type_dependent_expression_p (tree expression)
  type-dependent.  Checking this is important for functions with auto return
  type, which looks like a dependent type.  */
   if (TREE_CODE (expression) == FUNCTION_DECL
+  && DECL_LANG_SPECIFIC (expression)
   && !(DECL_CLASS_SCOPE_P (expression)
   && dependent_type_p (DECL_CONTEXT (expression)))
   && !(DECL_FRIEND_P (expression)
-- 
2.2.1


...v3/include/bits/cpp_type_traits.h:408:32: internal compiler error: 
Segmentation fault
 __miter_base(_Iterator __it)
^
0xd77fff crash_signal
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/toplev.c:326
0xd77fff crash_signal
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/toplev.c:326
0x74aeb9 type_dependent_expression_p(tree_node*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/cp/pt.c:24293
0x6cda47 mangle_decl_string
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/cp/mangle.c:3740
0x6cdca8 get_mangled_id
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/cp/mangle.c:3782
0x6cdf50 mangle_decl(tree_node*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/cp/mangle.c:3820
0x74aeb9 type_dependent_expression_p(tree_node*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/cp/pt.c:24293
0x6cda47 mangle_decl_string
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/cp/mangle.c:3740
0x6cdca8 get_mangled_id
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/cp/mangle.c:3782
0x6cdf50 mangle_decl(tree_node*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/cp/mangle.c:3820
0x105af90 decl_assembler_name(tree_node*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/tree.c:673
0xc7d1e8 build_libfunc_function(char const*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/optabs-libfuncs.c:736
0xc7d717 init_one_libfunc(char const*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/optabs-libfuncs.c:766
0xc7d79a set_optab_libfunc(optab_tag, machine_mode, char const*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/optabs-libfuncs.c:804
0x105af90 decl_assembler_name(tree_node*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/tree.c:673
0xc7d1e8 build_libfunc_function(char const*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/optabs-libfuncs.c:736
0xc7d717 init_one_libfunc(char const*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/optabs-libfuncs.c:766
0xc7d79a set_optab_libfunc(optab_tag, machine_mode, char const*)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/optabs-libfuncs.c:804
0x10d49bb mips_init_libfuncs

/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/config/mips/mips.c:13425
0xd7836c lang_dependent_init_target
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/toplev.c:1758
0xd7941c target_reinit()
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/toplev.c:1880
0x13e6f55 save_target_globals()
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/target-globals.c:86
0x10e0124 mips_set_compression_mode

/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/config/mips/mips.c:19518
0xa7ee1b invoke_set_current_function_hook
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/function.c:4783
0xa88aa7 invoke_set_current_function_hook
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/function.c:4918
0xa88aa7 allocate_struct_function(tree_node*, bool)
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/function.c:4893
0x10d49bb mips_init_libfuncs

/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/config/mips/mips.c:13425
0xd7836c lang_dependent_init_target
/scratch/mpf/overtest/64126/240294/shared/gcc/gcc/toplev.c:1758
0xd7941c target_reinit()
/scratch/mpf/over

RE: [PATCH] Fix ICE caused by a missing check for DECL_LANG_SPECIFIC

[Matthew Fortune]

Jason Merrill  writes:
> On Thu, Mar 1, 2018 at 7:02 AM, Matthew Fortune 
> wrote:
> > Hi,
> >
> > It seems we have had a bug for some time that causes an ICE and
> prevents the
> > MIPS16 library builds from completing but is likely unrelated to
> MIPS16.
> > The problem is when we call target_reinit and library functions get
> created
> > as shown in the call stack at the end of this message. The first
> builtin
> > that triggers the problem happens to be one of the MIPS16 helpers but
> I
> > don't think there is anything unique about it. The issue appeared
> after some
> > refactoring work in r253600 where code testing DECL_CLASS_SCOPE_P and
> > DECL_FRIEND_P was previously guarded by a check for
> DECL_LANG_SPECIFIC but
> > not after.
> >
> > https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00604.html
> >
> > I don’t know if this is the correct solution or whether we need to
> change the
> > way builtins are initialised in the MIPS backend but I suspect this
> fix
> > is the right way to go.
> >
> > Cc: Jason as author of the original change.
> >
> > Thanks,
> > Matthew
> >
> > gcc/cp/
> > * pt.c (type_dependent_expression_p): Add missing check for
> > DECL_LANG_SPECIFIC.
> > ---
> >  gcc/cp/pt.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index 7345119..c88304f 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -24635,6 +24635,7 @@ type_dependent_expression_p (tree expression)
> >   type-dependent.  Checking this is important for functions with
> auto return
> >   type, which looks like a dependent type.  */
> >if (TREE_CODE (expression) == FUNCTION_DECL
> > +  && DECL_LANG_SPECIFIC (expression)
> >&& !(DECL_CLASS_SCOPE_P (expression)
> >&& dependent_type_p (DECL_CONTEXT (expression)))
> >&& !(DECL_FRIEND_P (expression)
> 
> I think we want to go into this block when DECL_LANG_SPECIFIC is NULL.
> Does this also fix the issue for you?

Thanks. Yes, this fixes it too. I wasn't sure which of the accessors were
dependent on DECL_LANG_SPECIFIC so ended up with a sledgehammer. 

Matthew

RE: [PATCH 0/4] ASAN for MIPS (o32)

[Matthew Fortune]

Hans-Peter Nilsson  writes:
> All patches are together run through the gcc and g++ test-suites
> to check ASAN results for mipsisa32r2el-linux-gnu.  As of
> r258635 those results are on par with those for
> arm-linux-gnueabihf, so without delay I present the current
> state.  Maybe it's non-intrusive enough to be ok for gcc-8?
> MIPS maintainers (and interested party) CC:ed.

From a MIPS backend perspective all 4 patches are OK. I've done very
little to support upstream MIPS over this release so these
improvements are fantastic. I don't know the detail of asan support
so am going with the idea that your investigation has got to the
bottom of the problems; thanks for the detailed explanations.

I'm not sure I should really approve this for GCC-8 but rather ask
a global maintainer or Jakub/Richard as release managers given I
can't commit to do much to support the release and I won't want to
risk burdening others with a late change.

> For use with -fsanitize=address, you'll need a non-ancient glibc
> or equivalent (2002-ish), one that iterates on ELF headers for
> the EH info at exception time (rather, doesn't call
> __register_frame_info or __register_frame_info_bases at startup,
> ending up calling malloc/free) or else Asan will try to
> instrument the call to free and hang on a lock for eternity (or
> dies on a signal).  But that's no different than for other
> ports, AFAIU.
> 
> So, ok to commit?

As above, if a global maintainer is happy then yes.

Matthew

> 
> brgds, H-P

RE: [PATCH,Testsuite,MIPS] Fixing umips-stroe16-2.c failure started with r255348

[Matthew Fortune]

Hi Paul

> ChangeLog entries:
> 
> gcc/testsuite/ChangeLog
> 
> 2018-03-24  Chenghua Xu 
> 
> * gcc.target/mips/umips-stroe16-2.c: Change "length = 2"
>   to "l=2" in dg-final.

Looks good. Thanks for the cleanup. OK to commit.

Matthew

RE: [PATCH,Testsuite,MIPS] Fixing fix-r4000-n.c failure started with r255348

[Matthew Fortune]

Hi Paul,

> ChangeLog entries:
> 
> gcc/testsuite/ChangeLog
> 
> 2018-03-24  Chenghua Xu 
> 
> * gcc.target/mips/fix-r4000-1.c: Delete "[^\n]" in dg-final.
> * gcc.target/mips/fix-r4000-2.c: Likewise.
> * gcc.target/mips/fix-r4000-3.c: Likewise.
> * gcc.target/mips/fix-r4000-4.c: Likewise.
> * gcc.target/mips/fix-r4000-5.c: Likewise.
> * gcc.target/mips/fix-r4000-6.c: Likewise.
> * gcc.target/mips/fix-r4000-7.c: Likewise.
> * gcc.target/mips/fix-r4000-8.c: Likewise.
> * gcc.target/mips/fix-r4000-9.c: Likewise.
> * gcc.target/mips/fix-r4000-10.c: Likewise.
> * gcc.target/mips/fix-r4000-7.c: Change dg-final
>   "mulditi3_r4000" instead of "mulditi3".
> * gcc.target/mips/fix-r4000-8.c: Change dg-final
>   "umulditi3_r4000" instead of "umulditi3".

This looks good too. Another good cleanup, OK to commit.

Thanks,
Matthew

RE: [PATCH 1/2] MIPS/GCC/testsuite: Fix data-sym-pool.c for SVR4 model at -O0

[Matthew Fortune]

Maciej Rozycki  writes:
> Given that the SVR4 vs PLT code model consideration is irrelevant for
> this test case rather than rewriting the regular expression to match
> this variant of code just enforce the PLT model by using the `-mplt'
> option.  It is safe to use this option unconditionally as it is silently
> ignored with configurations that do not support this model, e.g. bare
> metal ELF.
> 
>   gcc/testsuite/
>   * gcc.target/mips/data-sym-pool.c (dg-options): Add `-mplt'.
> ---
> Hi,
> 
>  I have regression-tested this with the `mips-mti-linux-gnu' target and
> the o32, n32 and n64 ABIs.  The two latters are demoted to o64 by the
> test framework due to the lack of MIPS16 support for the hard-float
> variants of these ABIs and I don't have soft-float multilibs configured,
> so instead I have verified n32/soft-float and n64/soft-float variants by
> hand.  The latter revealed the need for 2/2.
> 
>  Finally I do not have a bare metal ELF configuration available for
> regression-testing right now, so I only verified that `-mplt' is
> silently ignored.  Code generated is expected to be the same as in the
> PLT mode.
> 
>  OK to apply?

Hi Maciej,

Sorry for skimming past this and being slow to respond. I agree with your
choice of forcing an option to stabilise the generated code it does tend
to future proof better than leaving options floating.

OK to apply but given the release date I've added RMs to approve for
trunk right now.

Thanks,
Matthew

RE: [PATCH 2/2] MIPS/GCC/testsuite: Fix data-sym-pool.c for n64 code

[Matthew Fortune]

Maciej Rozycki  writes:
> (we have no support for hard-float n64 MIPS16 code generation), which
> means that the test case will fail, as the regular expression pattern
> expects `lw' and `.word' rather than `ld' and `.dword' respectively to
> appear in assembly code generation.  Correct the pattern in an obvious
> way then making it accept both intructions and pseudo-ops.
> 
>   gcc/testsuite/
>   * gcc.target/mips/data-sym-pool.c (dg-options): Match `ld' and
>   `.dword' in addition to `lw' and `.word'.
> ---
> Hi,
> 
>  I have regression-tested this with the `mips-mti-linux-gnu' target and
> the o32 ABI.  I don't have an n64 soft-float multilib configured, so the
> manually generated assembly file included with the description serves as
> the proof for what needs to be matched.
> 
>  OK to apply?

Hi Maciej,

Apologies as before on slowness. This looks trivially OK to me but that's
thanks to the description, much appreciated.

OK for commit but adding RMs given the imminent release.

Thanks,
Matthew

RE: [PATCH] Add target hook to override DWARF2 frame register size

[Matthew Fortune]

> Please don't add target macros. Add a hook if you must, but we're
> supposed to remove target macros, not add new ones :-)

Thanks Steven, I wasn't sure if there were still things that were
acceptable as macros. There's a lot to get rid of still.

Updated patch using a target hook. I've opted to move the logic
which handles part clobbered registers into the default implementation
as that seemed natural. I have no real preference if others feel that
is the wrong thing to do. This will be used by an up-coming patch for
MIPS O32 ABI extensions.

Bootstrapped and regtested on x86_64-linux-gnu.

Thanks,
Matthew

gcc/
* target.def (TARGET_DWARF_FRAME_REG_MODE): New target hook.
* targhooks.c (default_dwarf_frame_reg_mode): New function.
* targhooks.h (default_dwarf_frame_reg_mode): New prototype.
* doc/tm.texi.in (TARGET_DWARF_FRAME_REG_MODE): Document.
* doc/tm.texi: Regenerate.
* dwarf2cfi.c (expand_builtin_init_dwarf_reg_sizes): Abstract mode
selection logic to default_dwarf_frame_reg_mode.
---
 gcc/doc/tm.texi|  7 +++
 gcc/doc/tm.texi.in |  2 ++
 gcc/dwarf2cfi.c|  4 +---
 gcc/target.def | 11 +++
 gcc/targhooks.c| 13 +
 gcc/targhooks.h|  1 +
 6 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index dd72b98..aa92ce4 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -6604,6 +6604,8 @@ the target supports DWARF 2 frame unwind information.
 
 @hook TARGET_DWARF_REGISTER_SPAN
 
+@hook TARGET_DWARF_FRAME_REG_MODE
+
 @hook TARGET_INIT_DWARF_REG_SIZES_EXTRA
 
 @hook TARGET_ASM_TTYPE
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index 85cfb60..a673106 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -271,11 +271,9 @@ expand_builtin_init_dwarf_reg_sizes (tree address)
   if (rnum < DWARF_FRAME_REGISTERS)
{
  HOST_WIDE_INT offset = rnum * GET_MODE_SIZE (mode);
- enum machine_mode save_mode = reg_raw_mode[i];
  HOST_WIDE_INT size;
+ enum machine_mode save_mode = targetm.dwarf_frame_reg_mode (i);
 
- if (HARD_REGNO_CALL_PART_CLOBBERED (i, save_mode))
-   save_mode = choose_hard_reg_mode (i, 1, true);
  if (dnum == DWARF_FRAME_RETURN_COLUMN)
{
  if (save_mode == VOIDmode)
diff --git a/gcc/target.def b/gcc/target.def
index 3a41db1..d5aba51 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -3216,6 +3216,17 @@ If not defined, the default is to return 
@code{NULL_RTX}.",
  rtx, (rtx reg),
  hook_rtx_rtx_null)
 
+/* Given a register return the mode of the corresponding DWARF frame
+   register.  */
+DEFHOOK
+(dwarf_frame_reg_mode,
+ "Given a register, this hook should return the mode which the\n\
+corresponding Dwarf frame register should have.  This is normally\n\
+used to return a smaller mode than the raw mode to prevent call\n\
+clobbered parts of a register altering the frame register size.",
+ enum machine_mode, (int regno),
+ default_dwarf_frame_reg_mode)
+
 /* If expand_builtin_init_dwarf_reg_sizes needs to fill in table
entries not corresponding directly to registers below
FIRST_PSEUDO_REGISTER, this hook should generate the necessary
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 0f27a5a..765bf3b 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1456,6 +1456,19 @@ default_debug_unwind_info (void)
   return UI_NONE;
 }
 
+/* Determine the correct mode for a Dwarf frame register that represents
+   register REGNO.  */
+
+enum machine_mode
+default_dwarf_frame_reg_mode (int regno)
+{
+  enum machine_mode save_mode = reg_raw_mode[regno];
+
+  if (HARD_REGNO_CALL_PART_CLOBBERED (regno, save_mode))
+save_mode = choose_hard_reg_mode (regno, 1, true);
+  return save_mode;
+}
+
 /* To be used by targets where reg_raw_mode doesn't return the right
mode for registers used in apply_builtin_return and apply_builtin_arg.  */
 
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 4be33f8..fa88679 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -194,6 +194,7 @@ extern int default_label_align_max_skip (rtx);
 extern int default_jump_align_max_skip (rtx);
 extern section * default_function_section(tree decl, enum node_frequency freq,
  bool startup, bool exit);
+extern enum machine_mode default_dwarf_frame_reg_mode (int);
 extern enum machine_mode default_get_reg_raw_mode (int);
 extern bool default_keep_leaf_when_profiled ();
 
-- 
1.9.4

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

[Matthew Fortune]

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?

Thanks,
Matthew

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

[Matthew Fortune]

Matthew Fortune  writes:
> 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.

I forgot to add that there will be some side effects to this patch which are
not ideal but also unavoidable. The main one I know of is the MIPS Linux
kernel which is compiled as soft-float as there must be an absolute guarantee
that there is no floating point register usage. However, there is assembly
code which uses the floating-point registers for context switches and these
modules have not been set up to use .set hardfloat. This means that all
existing kernel releases will not build with the new compiler. I have
asked some of the MIPS kernel developers to apply fixes to the relevant files
and back-port to any supported release. The code has always been notionally
wrong but never detected as the assembler did not know that it was assembling
soft-float code so allowed FP instructions. The fixed code will build with
old and new tools.

Similar issues will arise if anything is compiled as single-float but is
using double-precision instructions currently. The fixes are simple and it is
arguably an improvement to such code bases to find these issues and add
appropriate .set directives to account for the special usage.

Matthew

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

[Matthew Fortune]

Eric Christopher  writes:
> On Sat, Aug 9, 2014 at 12:00 PM, Matthew Fortune
>  wrote:
> > 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 one offhand, but in skimming the binutils patch I don't
> recall seeing anything that tested this combination. May have missed
> it though.
> 
> That said, the patch looks ok and if you'd like to add some tests for
> .set and the command line options to binutils that'd be great as well.

What sort of coverage do you think we need here? I did exhaustive coverage
of anything which can affect the ABI but don't think we need the same for
command-line options vs .module vs .set.

There were two pre-existing tests: mips-hard-float-flag and
mips-double-float-flag which pretty much test these cases I think. Is that
OK for now until we identify any other areas which need additional
coverage?

Matthew

RE: RE: RE: Re: [MIPS r5900] libgcc floating point fixes

[Matthew Fortune]

> > > > What is harder to fix about n32 than o32?
> > >
> > > "-msingle-float" with n32 creates 64 bit FPU instructions like dmtc1 and
> > > dmfc1. So I can't compile it for r5900. When I disable it, I get
> internal
> > > compiler errors.
> >
> > That certainly seems like a bug. Can you file a bug report for that with
> > a test case? I'll try and handle it when I next work on the single-float
> > support in GCC.
> 
> I added the bug report:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61983

Thanks.

> > Did you manage to get the libgcc patch reworked to be independent of
> r5900?
> 
> I needed to get my build environment in a state where I can change the code.
> I needed to test building the toolchain for 14 different MIPS
> configurations. I tested that 7 configurations build something where
> floating point calculations are correct. I needed to use my own tests for
> this, because I didn't find any testsuite which does it correctly.
> 
> I attached the new version of the patch which I tested with and without the
> FPU emulation on the MIPS r5900; i.e. I didn't tested with a normal MIPS,
> just what my Linux kernel emulates on r5900.

This seems better.

> When I compile for mipsel ABI o32 with -mhard-float and -msingle-fpu, It get
> the warning:
> 
> Warning: float register should be even, was 1.
> 
> I would say the warning is wrong, because odd FPU registers are available.

This is wrong but this is the area which is in flux at the moment. Can you
confirm which version of binutils you are using? Single-float support from both
GCC and binutils has been slightly inconsistent for a while and recent changes
to binutils have exposed the differences.

Matthew

[PATCH][MIPS] Do not reload unallocated FP_REGS pseudos via GR_REGS

[Matthew Fortune]

The secondary_reload_class hook gets called for pseudos which have
not been allocated a hard register. For pseudos with the FP_REGS
class this results in the hook stating that the pseudo must be
reloaded via GR_REGS as it is neither in an FP_REG nor in memory.
This is obviously wasteful as LRA will go on to allocate an FP_REG
and a GR_REG along with a move and load/store as required.

An example code sequence which is seen prior to the patch is:

lw $2, xxx
lw $3, xxx
mtc1 $2, $f0
mtc1 $3, $f1

and is replaced by:

ldc1 $f0, xxx

Similarly for stores and 4-byte values. Assembly output from
compare-all-tests shows that this is pretty much the only change
to generated code. There were 3 times as many lw/sw/mtc1/mfc1
instructions removed as compared to [ls][wd]c1 instructions
added. There was also some noise from different register
allocation but overall instruction counts remain very similar:

Added fp load store: 13631
Removed integer load/store/move: 41504
Added other: 14431
Removed other: 13244

This patch split out from:
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01815.html

OK to commit?

thanks,
Matthew

gcc/
* config/mips/mips.c (mips_secondary_reload_class): Handle
regno < 0 case.
---
 gcc/config/mips/mips.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index d8654c4..9bee4e6 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -12134,8 +12134,9 @@ mips_secondary_reload_class (enum reg_class rclass,
 
   if (reg_class_subset_p (rclass, FP_REGS))
 {
-  if (MEM_P (x)
- && (GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8))
+  if (regno < 0
+ || (MEM_P (x)
+ && (GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8)))
/* In this case we can use lwc1, swc1, ldc1 or sdc1.  We'll use
   pairs of lwc1s and swc1s if ldc1 and sdc1 are not supported.  */
return NO_REGS;
-- 
1.9.4

Ping - RE: [PATCH] Add target hook to override DWARF2 frame register size

[Matthew Fortune]

Ping.

Thanks, 
Matthew

> Sent: 07 August 2014 07:21
> > Please don't add target macros. Add a hook if you must, but we're
> > supposed to remove target macros, not add new ones :-)
> 
> Thanks Steven, I wasn't sure if there were still things that were
> acceptable as macros. There's a lot to get rid of still.
> 
> Updated patch using a target hook. I've opted to move the logic
> which handles part clobbered registers into the default implementation
> as that seemed natural. I have no real preference if others feel that
> is the wrong thing to do. This will be used by an up-coming patch for
> MIPS O32 ABI extensions.
> 
> Bootstrapped and regtested on x86_64-linux-gnu.
> 
> Thanks,
> Matthew
> 
> gcc/
>   * target.def (TARGET_DWARF_FRAME_REG_MODE): New target hook.
>   * targhooks.c (default_dwarf_frame_reg_mode): New function.
>   * targhooks.h (default_dwarf_frame_reg_mode): New prototype.
>   * doc/tm.texi.in (TARGET_DWARF_FRAME_REG_MODE): Document.
>   * doc/tm.texi: Regenerate.
>   * dwarf2cfi.c (expand_builtin_init_dwarf_reg_sizes): Abstract mode
>   selection logic to default_dwarf_frame_reg_mode.
> ---
>  gcc/doc/tm.texi|  7 +++
>  gcc/doc/tm.texi.in |  2 ++
>  gcc/dwarf2cfi.c|  4 +---
>  gcc/target.def | 11 +++
>  gcc/targhooks.c| 13 +
>  gcc/targhooks.h|  1 +
>  6 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index dd72b98..aa92ce4 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -6604,6 +6604,8 @@ the target supports DWARF 2 frame unwind
> information.
> 
>  @hook TARGET_DWARF_REGISTER_SPAN
> 
> +@hook TARGET_DWARF_FRAME_REG_MODE
> +
>  @hook TARGET_INIT_DWARF_REG_SIZES_EXTRA
> 
>  @hook TARGET_ASM_TTYPE
> diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
> index 85cfb60..a673106 100644
> --- a/gcc/dwarf2cfi.c
> +++ b/gcc/dwarf2cfi.c
> @@ -271,11 +271,9 @@ expand_builtin_init_dwarf_reg_sizes (tree address)
>if (rnum < DWARF_FRAME_REGISTERS)
>   {
> HOST_WIDE_INT offset = rnum * GET_MODE_SIZE (mode);
> -   enum machine_mode save_mode = reg_raw_mode[i];
> HOST_WIDE_INT size;
> +   enum machine_mode save_mode = targetm.dwarf_frame_reg_mode (i);
> 
> -   if (HARD_REGNO_CALL_PART_CLOBBERED (i, save_mode))
> - save_mode = choose_hard_reg_mode (i, 1, true);
> if (dnum == DWARF_FRAME_RETURN_COLUMN)
>   {
> if (save_mode == VOIDmode)
> diff --git a/gcc/target.def b/gcc/target.def
> index 3a41db1..d5aba51 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -3216,6 +3216,17 @@ If not defined, the default is to return
> @code{NULL_RTX}.",
>   rtx, (rtx reg),
>   hook_rtx_rtx_null)
> 
> +/* Given a register return the mode of the corresponding DWARF frame
> +   register.  */
> +DEFHOOK
> +(dwarf_frame_reg_mode,
> + "Given a register, this hook should return the mode which the\n\
> +corresponding Dwarf frame register should have.  This is normally\n\
> +used to return a smaller mode than the raw mode to prevent call\n\
> +clobbered parts of a register altering the frame register size.",
> + enum machine_mode, (int regno),
> + default_dwarf_frame_reg_mode)
> +
>  /* If expand_builtin_init_dwarf_reg_sizes needs to fill in table
> entries not corresponding directly to registers below
> FIRST_PSEUDO_REGISTER, this hook should generate the necessary
> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> index 0f27a5a..765bf3b 100644
> --- a/gcc/targhooks.c
> +++ b/gcc/targhooks.c
> @@ -1456,6 +1456,19 @@ default_debug_unwind_info (void)
>return UI_NONE;
>  }
> 
> +/* Determine the correct mode for a Dwarf frame register that
> represents
> +   register REGNO.  */
> +
> +enum machine_mode
> +default_dwarf_frame_reg_mode (int regno)
> +{
> +  enum machine_mode save_mode = reg_raw_mode[regno];
> +
> +  if (HARD_REGNO_CALL_PART_CLOBBERED (regno, save_mode))
> +save_mode = choose_hard_reg_mode (regno, 1, true);
> +  return save_mode;
> +}
> +
>  /* To be used by targets where reg_raw_mode doesn't return the right
> mode for registers used in apply_builtin_return and
> apply_builtin_arg.  */
> 
> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> index 4be33f8..fa88679 100644
> --- a/gcc/targhooks.h
> +++ b/gcc/targhooks.h
> @@ -194,6 +194,7 @@ extern int default_label_align_max_skip (rtx);
>  extern int default_jump_align_max_skip (rtx);
>  extern section * default_function_section(tree decl, enum
> node_frequency freq,
> bool startup, bool exit);
> +extern enum machine_mode default_dwarf_frame_reg_mode (int);
>  extern enum machine_mode default_get_reg_raw_mode (int);
>  extern bool default_keep_leaf_when_profiled ();
> 
> --
> 1.9.4

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

[Matthew Fortune]

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_MODE): Define target hook.
* config/mips/mips.h (TARGET_FLOAT32): New macro.
(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/mip

Ping^2 - RE: [PATCH] Add target hook to override DWARF2 frame register size

[Matthew Fortune]

Ping^2

Added Jason as maintainer for dwarf related things.

This hook will be used in the following patch:
https://gcc.gnu.org/ml/gcc-patches/2014-08/msg02172.html

Thanks,
Matthew

> Ping.
> 
> Thanks,
> Matthew
> 
> > Sent: 07 August 2014 07:21
> > > Please don't add target macros. Add a hook if you must, but we're
> > > supposed to remove target macros, not add new ones :-)
> >
> > Thanks Steven, I wasn't sure if there were still things that were
> > acceptable as macros. There's a lot to get rid of still.
> >
> > Updated patch using a target hook. I've opted to move the logic
> > which handles part clobbered registers into the default implementation
> > as that seemed natural. I have no real preference if others feel that
> > is the wrong thing to do. This will be used by an up-coming patch for
> > MIPS O32 ABI extensions.
> >
> > Bootstrapped and regtested on x86_64-linux-gnu.
> >
> > Thanks,
> > Matthew
> >
> > gcc/
> > * target.def (TARGET_DWARF_FRAME_REG_MODE): New target hook.
> > * targhooks.c (default_dwarf_frame_reg_mode): New function.
> > * targhooks.h (default_dwarf_frame_reg_mode): New prototype.
> > * doc/tm.texi.in (TARGET_DWARF_FRAME_REG_MODE): Document.
> > * doc/tm.texi: Regenerate.
> > * dwarf2cfi.c (expand_builtin_init_dwarf_reg_sizes): Abstract mode
> > selection logic to default_dwarf_frame_reg_mode.
> > ---
> >  gcc/doc/tm.texi|  7 +++
> >  gcc/doc/tm.texi.in |  2 ++
> >  gcc/dwarf2cfi.c|  4 +---
> >  gcc/target.def | 11 +++
> >  gcc/targhooks.c| 13 +
> >  gcc/targhooks.h|  1 +
> >  6 files changed, 35 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> > index dd72b98..aa92ce4 100644
> > --- a/gcc/doc/tm.texi.in
> > +++ b/gcc/doc/tm.texi.in
> > @@ -6604,6 +6604,8 @@ the target supports DWARF 2 frame unwind
> > information.
> >
> >  @hook TARGET_DWARF_REGISTER_SPAN
> >
> > +@hook TARGET_DWARF_FRAME_REG_MODE
> > +
> >  @hook TARGET_INIT_DWARF_REG_SIZES_EXTRA
> >
> >  @hook TARGET_ASM_TTYPE
> > diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
> > index 85cfb60..a673106 100644
> > --- a/gcc/dwarf2cfi.c
> > +++ b/gcc/dwarf2cfi.c
> > @@ -271,11 +271,9 @@ expand_builtin_init_dwarf_reg_sizes (tree
> address)
> >if (rnum < DWARF_FRAME_REGISTERS)
> > {
> >   HOST_WIDE_INT offset = rnum * GET_MODE_SIZE (mode);
> > - enum machine_mode save_mode = reg_raw_mode[i];
> >   HOST_WIDE_INT size;
> > + enum machine_mode save_mode = targetm.dwarf_frame_reg_mode (i);
> >
> > - if (HARD_REGNO_CALL_PART_CLOBBERED (i, save_mode))
> > -   save_mode = choose_hard_reg_mode (i, 1, true);
> >   if (dnum == DWARF_FRAME_RETURN_COLUMN)
> > {
> >   if (save_mode == VOIDmode)
> > diff --git a/gcc/target.def b/gcc/target.def
> > index 3a41db1..d5aba51 100644
> > --- a/gcc/target.def
> > +++ b/gcc/target.def
> > @@ -3216,6 +3216,17 @@ If not defined, the default is to return
> > @code{NULL_RTX}.",
> >   rtx, (rtx reg),
> >   hook_rtx_rtx_null)
> >
> > +/* Given a register return the mode of the corresponding DWARF frame
> > +   register.  */
> > +DEFHOOK
> > +(dwarf_frame_reg_mode,
> > + "Given a register, this hook should return the mode which the\n\
> > +corresponding Dwarf frame register should have.  This is normally\n\
> > +used to return a smaller mode than the raw mode to prevent call\n\
> > +clobbered parts of a register altering the frame register size.",
> > + enum machine_mode, (int regno),
> > + default_dwarf_frame_reg_mode)
> > +
> >  /* If expand_builtin_init_dwarf_reg_sizes needs to fill in table
> > entries not corresponding directly to registers below
> > FIRST_PSEUDO_REGISTER, this hook should generate the necessary
> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> > index 0f27a5a..765bf3b 100644
> > --- a/gcc/targhooks.c
> > +++ b/gcc/targhooks.c
> > @@ -1456,6 +1456,19 @@ default_debug_unwind_info (void)
> >return UI_NONE;
> >  }
> >
> > +/* Determine the correct mode for a Dwarf frame register that
> > represents
> > +   register REGNO.  */
> > +
> > +enum machine_mode
> > +default_dwarf_frame_reg_mode (int regno)
> > +{
> > +  enum machine_mode save_mode = reg_raw_mode[regno];
> > +
> > +  if (HARD_REGNO_CALL_PART_CLOBBERED (regno, save_mode))
> > +save_mode = choose_hard_reg_mode (regno, 1, true);
> > +  return save_mode;
> > +}
> > +
> >  /* To be used by targets where reg_raw_mode doesn't return the right
> > mode for registers used in apply_builtin_return and
> > apply_builtin_arg.  */
> >
> > diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> > index 4be33f8..fa88679 100644
> > --- a/gcc/targhooks.h
> > +++ b/gcc/targhooks.h
> > @@ -194,6 +194,7 @@ extern int default_label_align_max_skip (rtx);
> >  extern int default_jump_align_max_skip (rtx);
> >  extern section * default_function_section(tree decl, enum
> > node_frequency freq,
> >   bool startup, bool exit);
> > +e

RE: Ping^2 - RE: [PATCH] Add target hook to override DWARF2 frame register size

[Matthew Fortune]

> On 09/02/2014 01:59 AM, Matthew Fortune wrote:
> >> > gcc/
> >> >  * target.def (TARGET_DWARF_FRAME_REG_MODE): New target hook.
> >> >  * targhooks.c (default_dwarf_frame_reg_mode): New function.
> >> >  * targhooks.h (default_dwarf_frame_reg_mode): New prototype.
> >> >  * doc/tm.texi.in (TARGET_DWARF_FRAME_REG_MODE): Document.
> >> >  * doc/tm.texi: Regenerate.
> >> >  * dwarf2cfi.c (expand_builtin_init_dwarf_reg_sizes): Abstract mode
> >> >  selection logic to default_dwarf_frame_reg_mode.
> 
> Ok.

Thanks, committed as r214898.

Matthew

RE: [PATCH][MIPS] Do not reload unallocated FP_REGS pseudos via GR_REGS

[Matthew Fortune]

Ping!

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-ow...@gcc.gnu.org]
> On Behalf Of Matthew Fortune
> Sent: 18 August 2014 14:25
> To: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> Cc: Richard Sandiford; Eric Christopher (echri...@gmail.com)
> Subject: [PATCH][MIPS] Do not reload unallocated FP_REGS pseudos via GR_REGS
> 
> The secondary_reload_class hook gets called for pseudos which have
> not been allocated a hard register. For pseudos with the FP_REGS
> class this results in the hook stating that the pseudo must be
> reloaded via GR_REGS as it is neither in an FP_REG nor in memory.
> This is obviously wasteful as LRA will go on to allocate an FP_REG
> and a GR_REG along with a move and load/store as required.
> 
> An example code sequence which is seen prior to the patch is:
> 
> lw $2, xxx
> lw $3, xxx
> mtc1 $2, $f0
> mtc1 $3, $f1
> 
> and is replaced by:
> 
> ldc1 $f0, xxx
> 
> Similarly for stores and 4-byte values. Assembly output from
> compare-all-tests shows that this is pretty much the only change
> to generated code. There were 3 times as many lw/sw/mtc1/mfc1
> instructions removed as compared to [ls][wd]c1 instructions
> added. There was also some noise from different register
> allocation but overall instruction counts remain very similar:
> 
> Added fp load store: 13631
> Removed integer load/store/move: 41504
> Added other: 14431
> Removed other: 13244
> 
> This patch split out from:
> https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01815.html
> 
> OK to commit?
> 
> thanks,
> Matthew
> 
> gcc/
>   * config/mips/mips.c (mips_secondary_reload_class): Handle
>   regno < 0 case.
> ---
>  gcc/config/mips/mips.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index d8654c4..9bee4e6 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -12134,8 +12134,9 @@ mips_secondary_reload_class (enum reg_class rclass,
> 
>if (reg_class_subset_p (rclass, FP_REGS))
>  {
> -  if (MEM_P (x)
> -   && (GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8))
> +  if (regno < 0
> +   || (MEM_P (x)
> +   && (GET_MODE_SIZE (mode) == 4 || GET_MODE_SIZE (mode) == 8)))
>   /* In this case we can use lwc1, swc1, ldc1 or sdc1.  We'll use
>  pairs of lwc1s and swc1s if ldc1 and sdc1 are not supported.  */
>   return NO_REGS;
> --
> 1.9.4

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

[Matthew Fortune]

Ping!

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-ow...@gcc.gnu.org]
> On Behalf Of Matthew Fortune
> Sent: 22 August 2014 10:43
> To: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org); Eric Christopher
> (echri...@gmail.com)
> Cc: Moore, Catherine (catherine_mo...@mentor.com); Richard Sandiford; Rich
> Fuhler; ma...@codesourcery.com; Joseph Myers (jos...@codesourcery.com)
> 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_MODE): Define target hook.

RE: [Patch, MIPS] Cleanup mips header files.

[Matthew Fortune]

Hi Steve,

You're the lucky recipient of my first review so apologies for being
slow and cautious...

I tried to find a reason why the files were originally separated like this
and I can't see anything obvious.  I assume you also found no reason.
Presumably the separation was just to avoid disturbing the 32-bit configs
but I think it is a sensible move to merge them.

> --- a/gcc/config/mips/gnu-user.h
> +++ b/gcc/config/mips/gnu-user.h
> @@ -52,16 +52,20 @@ along with GCC; see the file COPYING3.  If not see
>  #undef MIPS_DEFAULT_GVALUE
>  #define MIPS_DEFAULT_GVALUE 0
> 
> -/* Borrowed from sparc/linux.h */
>  #undef GNU_USER_TARGET_LINK_SPEC
> -#define GNU_USER_TARGET_LINK_SPEC \
> - "%(endian_spec) \
> -  %{shared:-shared} \
> +#define GNU_USER_TARGET_LINK_SPEC "\
> +%{G*} %{EB} %{EL} %{!EB:%{!EL:%(endian_spec)}} %{mips*} \
> +%{shared} \

Indent two lines above. The '%{!EB:%{!EL:%(endian_spec)}}' is
redundant as it is covered by the DRIVER_SELF_SPEC which you have added
below. The linker won't accept -mips16 so you should use
MIPS_ISA_LEVEL_OPTION_SPEC to avoid passing mips16. Although we still
end up with an explicit list at least there is just one copy to maintain.

>%{!shared: \
>  %{!static: \
>%{rdynamic:-export-dynamic} \
> -  -dynamic-linker " GNU_USER_DYNAMIC_LINKER "} \
> -  %{static:-static}}"
> +  %{mabi=n32: -dynamic-linker " GNU_USER_DYNAMIC_LINKERN32 "} \
> +  %{mabi=64: -dynamic-linker " GNU_USER_DYNAMIC_LINKER64 "} \
> +  %{mabi=32: -dynamic-linker " GNU_USER_DYNAMIC_LINKER32 "}} \
> +%{static:-static}} \

Very much a nit but if we are going to have %{shared} above then we should
have just %{static} here.

> +%{mabi=n32:-m" GNU_USER_LINK_EMULATIONN32 "} \
> +%{mabi=64:-m" GNU_USER_LINK_EMULATION64 "} \
> +%{mabi=32:-m" GNU_USER_LINK_EMULATION32 "}"
>  #undef LINK_SPEC
>  #define LINK_SPEC GNU_USER_TARGET_LINK_SPEC
> 
> @@ -122,7 +126,9 @@ extern const char *host_detect_local_cpu (int argc,
> const char **argv);
>   specs handling by removing a redundant option.  */  
> \
>"%{!mno-shared:%/* -mplt likewise has no effect for -mabi=64 without -msym32.  */  \
> -  "%{mabi=64:%{!msym32:% +  "%{mabi=64:%{!msym32:% +  " %{!EB:%{!EL:%(endian_spec)}}"\
> +  " %{!mabi=*: -" MULTILIB_ABI_DEFAULT "}"

Another nit, since this code is being changed can you update this from using
a leading space to be a comma separated list of strings like this:

> +  "%{mabi=64:%{!msym32:% +  "%{!EB:%{!EL:%(endian_spec)}}",\
> +  "%{!mabi=*: -" MULTILIB_ABI_DEFAULT "}"

With those changes can you double check that a default big and default little
endian build pass -EB/-EL respectively by default and are changed when using
-EL/-EB explicitly? There should only be one -E* option passed to the linker
theoretically, unless multiple explicit -E* options are given on the command
line.

Otherwise OK (assuming the link specs behave as described above).

Thanks,
Matthew

RE: [Patch, MIPS] Add .note.GNU-stack section

[Matthew Fortune]

> I talked to Andrew about what files he changed in GCC and created and
> tested this new patch.  Andrew also mentioned changing some assembly
> files in glibc but I don't see any use of '.section .note.GNU-stack' in
> any assembly files in glibc (for any platform) so I wasn't planning on
> creating a glibc to add them to mips glibc assembly language files.
> 
> OK to check in this patch?

I think we need to wait for some conclusion about how the kernel will
cope with a non-executable stack before committing this. The discussion
is progressing on the glibc and kernel lists.

As far as I can tell the kernel will respond to the PT_GNU_STACK
segment already for MIPS and then lead to a crash if the FPU emulator is
invoked. If this is the case then it would be ideal if we can somehow
prevent a stack becoming non-executable unless built against a kernel
which is known to be safe for no-execstack.

Existing versions of glibc will simply follow suit with the compiler
and apply --noexecstack when building assembly files if the compiler
emits a .note.GNU-stack section. Given that behaviour then as soon
as we enable no-execstack support in the compiler then we will get
PT_GNU_STACK on the binaries and risk a crash.

I'm struggling to come up with a good way to make this safe on existing
kernels. One possibility is that MIPS would have to implement the
compiler driven no-execstack support by passing the --no-execstack
option to the assembler instead of using the .note.GNU-stack section
explicitly. This would avoid triggering the configure check in glibc
and allow us to write a MIPS specific check in glibc which looks at
both the kernel version and whether the compiler enables no-execstack.

Slightly convoluted but safe. I'm not sure how uclibc and musl deal
with this issue but we should really check them too (but they are still
secondary to glibc I think). Any other ideas?

Matthew

> 
> Steve Ellcey
> sell...@mips.com
> 
> 
> 
> 2014-09-26  Steve Ellcey  
> 
>   * config/mips/mips.c (TARGET_ASM_FILE_END): Define.
>   * libgcc/config/mips/mips16.S: Add .note.GNU-stack section.
>   * libgcc/config/mips/vr4120-div.S: Ditto.
>   * libgcc/config/mips/crti.S: Ditto.
>   * libgcc/config/mips/crtn.S: Ditto.
> 
> 
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index f9713c1..39020d7 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -19146,6 +19146,9 @@ mips_lra_p (void)
>  #undef TARGET_LRA_P
>  #define TARGET_LRA_P mips_lra_p
> 
> +#undef TARGET_ASM_FILE_END
> +#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
> +
>  struct gcc_target targetm = TARGET_INITIALIZER;
>  

>  #include "gt-mips.h"
> diff --git a/libgcc/config/mips/crti.S b/libgcc/config/mips/crti.S
> index 6980594..93436c0 100644
> --- a/libgcc/config/mips/crti.S
> +++ b/libgcc/config/mips/crti.S
> @@ -21,6 +21,10 @@ a copy of the GCC Runtime Library Exception along with
> this program;
>  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>  .  */
> 
> +
> +/* An executable stack is *not* required for these functions.  */
> + .section .note.GNU-stack,"",%progbits
> +
>  /* 4 slots for argument spill area.  1 for cpreturn, 1 for stack.
> Return spill offset of 40 and 20.  Aligned to 16 bytes for n32.  */
> 
> diff --git a/libgcc/config/mips/crtn.S b/libgcc/config/mips/crtn.S
> index 0de2d0c..6f2c301 100644
> --- a/libgcc/config/mips/crtn.S
> +++ b/libgcc/config/mips/crtn.S
> @@ -21,6 +21,9 @@ a copy of the GCC Runtime Library Exception along with
> this program;
>  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>  .  */
> 
> +/* An executable stack is *not* required for these functions.  */
> +.section .note.GNU-stack,"",%progbits
> +
>  /* 4 slots for argument spill area.  1 for cpreturn, 1 for stack.
> Return spill offset of 40 and 20.  Aligned to 16 bytes for n32.  */
> 
> diff --git a/libgcc/config/mips/mips16.S b/libgcc/config/mips/mips16.S
> index dde8939..58e4377 100644
> --- a/libgcc/config/mips/mips16.S
> +++ b/libgcc/config/mips/mips16.S
> @@ -35,6 +35,10 @@ see the files COPYING3 and COPYING.RUNTIME
> respectively.  If not, see
> values using the soft-float calling convention, but do the actual
> operation using the hard floating point instructions.  */
> 
> +/* An executable stack is *not* required for these functions.  */
> +.section .note.GNU-stack,"",%progbits
> + .previous
> +
>  #if defined _MIPS_SIM && (_MIPS_SIM == _ABIO32 || _MIPS_SIM == _ABIO64)
> 
>  /* This file contains 32-bit assembly code.  */
> diff --git a/libgcc/config/mips/vr4120-div.S b/libgcc/config/mips/vr4120-
> div.S
> index 76c4e7a..664d3c3 100644
> --- a/libgcc/config/mips/vr4120-div.S
> +++ b/libgcc/config/mips/vr4120-div.S
> @@ -26,6 +26,10 @@ see the files COPYING3 and COPYING.RUNTIME
> respectively.  If not, see
> -mfix-vr4120.  div and ddiv do not give the corr

RE: [Patch, MIPS] Cleanup mips header files.

[Matthew Fortune]

> 2014-10-08  Steve Ellcey  
> 
>   * config/mips/mti-linux.h (DRIVER_SELF_SPECS): Change
>   LINUX64_DRIVER_SELF_SPECS to LINUX_DRIVER_SELF_SPECS

OK. I'd agree with obvious here.

Matthew

RE: [Patch, MIPS] Add Octeon3 support

[Matthew Fortune]

> 2014-10-08  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.

Sorry for the delay.  Looks OK just a couple of questions...

Is it intentional that you have not updated driver-native.c to
detect an Octeon 3 CPU? I guess you may be waiting on Octeon 3
being committed to the kernel so you know for sure how the
CPU will appear in procinfo?

Could you confirm what testing the patch has had?

Thanks,
Matthew

RE: [PATCH v2 0-6/11] Fix PR/61114, make direct vector reductions endianness-neutral

[Matthew Fortune]

Alan Lawrence  writes:
> Patches 7-11 migrate migrate ARM, x86, IA64 (I think), and mostly PowerPC,
> to
> the new reduc_(plus|[us](min|max))_scal_optab. I have not managed to work
> out
> how to do the same for MIPS (specifically what I need to add to
> mips_expand_vec_reduc), and have had no response from the maintainers, so
> am

Sorry, I was looking at this but failed to send an email saying so. The lack
of vec_extract appears to be the stumbling point here so at the very least
we need to add a naïve version of that I believe.

> (2) also renaming reduc_..._scal_optab back to reduc_..._optab; would
> break the
> MIPS backend if something were not done with it's existing patterns.

I suspect we can deal with this in time to make a rename OK.

One thing occurred to me about this change in general which is that on the
whole the reduction to a scalar seems good for an epilogue but is there
a problem if the result is then replicated across a vector for further
processing. I.e. a vector is reduced to a scalar, which moves the value
from a SIMD register to a GP register (because scalar modes are not
supported in SIMD registers generally) and then gets moved back to a
SIMD register to form part of a new vector? Would you expect the
redundant moves to get eliminated?

Thanks,
Matthew

RE: [MIPS] RFA: Use new rtl iterators in mips_rewrite_small_data

[Matthew Fortune]

> gcc/
>   * config/mips/mips.c (mips_rewrite_small_data_1): Take the context
>   as a parameter instead of the containing MEM.  Iterate over all
>   subrtxes.  Don't return a value.
>   (mips_rewrite_small_data): Update call accordingly.

OK

RE: [MIPS] RFA: Use new rtl iterators in mips_small_data_pattern_p

[Matthew Fortune]

> gcc/
>   * config/mips/mips.c: Include rtl-iter.h.
>   (mips_small_data_pattern_1): Take an rtx rather than an rtx pointer.
>   Take the context as a parameter instead of the containing MEM.
>   Iterate over all subrtxes.
>   (mips_small_data_pattern_p): Update call accordingly.

OK

RE: [MIPS] RFA: Use new rtl iterators in mips16_rewrite_pool_refs

[Matthew Fortune]

> gcc/
>   * config/mips/mips.c (mips16_rewrite_pool_refs_info): Delete.
>   (mips16_rewrite_pool_refs): Take the insn and constant pool as
>   parameters.  Iterate over the instruction's pattern and return void.
>   (mips16_lay_out_constants): Update accordingly.

OK
 

RE: [MIPS] RFA: Use new rtl iterators in r10k_needs_protection_p_1

[Matthew Fortune]

> gcc/
>   * config/mips/mips.c (r10k_needs_protection_p_1): Take an rtx
>   rather than an rtx pointer.  Change type of insn from "void *"
>   to its real type.  Return bool rather than int.  Iterate over
>   all subrtxes here.
>   (r10k_needs_protection_p_store): Update accordingly.
>   (r10k_needs_protection_p): Likewise.

OK

RE: [MIPS] RFA: Use new rtl iterators in mips_kernel_reg_p

[Matthew Fortune]

> gcc/
>   * config/mips/mips.c (mips_kernel_reg_p): Replace with...
>   (mips_refers_to_kernel_reg_p): ...this new function.
>   (mips_expand_prologue): Update accordingly.

OK

RE: [MIPS] RFA: Use new rtl iterators in mips_sim_wait_regs_1

[Matthew Fortune]

> gcc/
>   * config/mips/mips.c (mips_sim_insn): Update comment.
>   (mips_sim_wait_regs_2): Delete.
>   (mips_sim_wait_regs_1): Use FOR_EACH_SUBRTX_VAR.

OK

RE: [MIPS] RFA: Use new rtl iterators in mips_record_lo_sums

[Matthew Fortune]

> gcc/
>   * config/mips/mips.c (mips_record_lo_sum): Replace with...
>   (mips_record_lo_sums): ...this new function.
>   (mips_reorg_process_insns): Update accordingly.

OK

RE: [MIPS] RFA: Use new rtl iterators in r10k_needs_protection_p_call

[Matthew Fortune]

> gcc/
>   * config/mips/mips.c (r10k_needs_protection_p_call): Take a const_rtx
>   and return a bool.  Iterate over all subrtxes here.
>   (r10k_needs_protection_p): Update accordingly.

OK

RE: [MIPS] RFA: Use new rtl iterators in mips_need_noat_wrapper_p

[Matthew Fortune]

> gcc/
>   * config/mips/mips.c (mips_at_reg_p): Delete.
>   (mips_need_noat_wrapper_p): Use FOR_EACH_SUBRTX.

OK. That should be the last one to cover all changes to use
new rtl iterators for MIPS.

Thanks for splitting this up per change it made it easy to read
through.

Matthew

RE: [PATCH, ifcvt] Check size cost in noce_try_store_flag_mask

[Matthew Fortune]

Zhenqiang Chen  writes:
> For CSiBE, ARM Cortex-m0 result is a little better. A little regression
> for
> MIPS. Roughly no change for PowerPC.

Do I take it that a little regression for MIPS is so small it can be
considered noise? I haven't had chance to run CSiBE to see the difference.

Thanks,
Matthew

> 
> OK for trunk?
> 
> Thanks!
> -Zhenqiang
> 
> ChangeLog:
> 2014-10-27  Zhenqiang Chen  
> 
>   * ifcvt.c (noce_try_store_flag_mask): Check rtx cost.
> 
> testsuite/ChangeLog:
> 2014-10-27  Zhenqiang Chen  
> 
>   * gcc.target/arm/ifcvt-size-check.c: New test.
> 
> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index 949d2b4..3abd518 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -1393,6 +1393,14 @@ noce_try_store_flag_mask (struct noce_if_info
> *if_info)
> if (!seq)
>   return FALSE;
> 
> +   if (optimize_function_for_size_p (cfun))
> + {
> +   int old_cost = COSTS_N_INSNS (if_info->branch_cost + 1);
> +   int new_cost = seq_cost (seq, 0);
> +   if (new_cost > old_cost)
> + return FALSE;
> + }
> +
> emit_insn_before_setloc (seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a));
> return TRUE;
> diff --git a/gcc/testsuite/gcc.target/arm/ifcvt-size-check.c
> b/gcc/testsuite/gcc.target/arm/ifcvt-size-check.c
> new file mode 100644
> index 000..43fa16b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/ifcvt-size-check.c
> @@ -0,0 +1,13 @@
> +/* { dg-do assemble } */
> +/* { dg-options "-mthumb -Os " }  */
> +/* { dg-require-effective-target arm_thumb1_ok } */
> +
> +int
> +test (unsigned char iov_len, int count, int i)
> +{
> +  unsigned char bytes = 0;
> +  if ((unsigned char) ((char) 127 - bytes) < iov_len)
> +return 22;
> +  return 0;
> +}
> +/* { dg-final { object-size text <= 12 } } */
> 
> 

RE: [PATCH][MIPS] Implement O32 FPXX ABI (GCC)

[Matthew Fortune]

Hi Richard,

I've realised that I may need to do 'something' to prevent GCC from loading or
storing DFmode/DImode values to/from FPRs using pairs of SWC1/LWC1 when using
an unaligned address. Initial tests show that when loading from an unaligned
address (4-byte aligned) then GCC loads the two halves of a 64-bit value into
GPRs and then moves across to FPRs. This is good but I don't know if it is
guaranteed.

From what I can tell the backend doesn't specifically deal with loading
unaligned data but rather the normal load/store patterns are used by the
middle end. As such I'm not sure there is anything to prevent direct loads
to FPRs by parts.

Do you know one way or the other if unaligned doubles can currently be loaded
via pairs of lwc1 (same for store) and if so can you advise on an approach I 
could try to prevent this for FPXX? I will try to figure this out on my own in
the meantime.

Regards,
Matthew 

RE: [PATCH][MIPS] Implement O32 FPXX ABI (GCC)

[Matthew Fortune]

Richard Sandiford  writes:
> Matthew Fortune  writes:
> > I've realised that I may need to do 'something' to prevent GCC from
> loading or
> > storing DFmode/DImode values to/from FPRs using pairs of SWC1/LWC1
> when using
> > an unaligned address. Initial tests show that when loading from an
> unaligned
> > address (4-byte aligned) then GCC loads the two halves of a 64-bit
> value into
> > GPRs and then moves across to FPRs. This is good but I don't know if
> it is
> > guaranteed.
> >
> > From what I can tell the backend doesn't specifically deal with
> loading
> > unaligned data but rather the normal load/store patterns are used by
> the
> > middle end. As such I'm not sure there is anything to prevent direct
> loads
> > to FPRs by parts.
> >
> > Do you know one way or the other if unaligned doubles can currently be
> loaded
> > via pairs of lwc1 (same for store) and if so can you advise on an
> approach I
> > could try to prevent this for FPXX? I will try to figure this out on
> my own in
> > the meantime.
> 
> The port does handle some widths of unaligned access via the
> {insv,extv,extzv}misalign patterns.  That's how an unaligned DImode
> value will be handled on 64-bit targets.
> 
> The MIPS *misalign patterns only handle integer modes, so for other
> types of mode the target-independent code will fall back to using an
> integer load followed by a subreg (or a subreg followed by an integer
> store).  IIRC that's how an unaligned DFmode access will be handled on
> 64-bit targets.
> 
> For modes that are larger or smaller than *misalign can handle,
> the target-independent code has to split the access up into smaller
> pieces and reassemble them.  And these pieces have to have integer
> modes.
> E.g. on 32-bit targets a 4-byte-misaligned load into (reg:DF x) could be
> done by loading (subreg:SI (reg:DF x) 0) and (subreg:SI (reg:DF x) 4).
> The thing that prevents these individual loads from using LWC1 is
> CANNOT_CHANGE_MODE_CLASS, which (among other things) makes it invalid
> for any target-independent code to reduce a subreg of an FPR pair
> to an individual FPR.
> 
> [FWIW, the reason MIPS doesn't define {insv,extv,extzv}misalign for
> things
> like DImode on 32-bit targets is because there's no special architecture
> feature than can be used.  It's just a case of decomposing the access.
> Since that's a general technique, we want to make the target-independent
> code do it as well as possible rather than handle it in a port-specific
> way.]
> 
> So yeah, the combination of (a) STRICT_ALIGNMENT, (b) the lack of
> unaligned
> floating-point load/store patterns and (c) CANNOT_CHANGE_MODE_CLASS
> should
> guarantee what you want.

Thanks for all the details. I did not know if CANNOT_CHANGE_MODE_CLASS
would give this guarantee as I am conscious of MIPS 1 having to do pairs
of LWC1/SWC1 for doubles. However, if I understand the code correctly
the LWC1/SWC1 pairs are generated by splits for MIPS 1 and not directly
from target-independent code so CANNOT_CHANGE_MODE_CLASS does not impact
that explicit splitting logic. Is that right?

Regards,
Matthew

RE: [PATCH][MIPS] Implement O32 FPXX ABI (GCC)

[Matthew Fortune]

Richard Sandiford  writes:
> Matthew Fortune  writes: 
> > MIPSr6 only supports 64-bit registers which naturally leads to needing
> > -mfp64. MIPSr6 does however also support a single-precision only
> variant.
> >
> > For a single purpose native toolchain then --with-fp=64 can be used
> xor
> > --with-fpu=single to get the desired tools.
> 
> Hmm, does that really mean that an FPU that only implements S/W-format
> instructions must still have 64-bit registers?  And are MTHC1 and MFHC1

No, the single-precision only is 32-bit registers as you would assume.

> therefore always supported for single-float-only FPUs?  The r6
> documentation
> I downloaded made it sound like 32-bit FPUs were still allowed for
> S/W-only FPUs but that 64-bit FPUs are required if D/L-format
> instructions are supported.
> 
> Or is this more a single-precision+MSA thing?

No it is me overcomplicating things. We can deal with it as part of the
R6 patch. I have however modified the mti vendor specs to infer -mfpxx
for specific architectures rather than for everything other than mips1
to make it ready for R6.

> (Can't really respond to the later specs stuff without understanding
> this part first.)
> 
> >> >> >> > diff --git a/gcc/testsuite/gcc.target/mips/call-clobbered-2.c
> >> >> >> b/gcc/testsuite/gcc.target/mips/call-clobbered-2.c
> >> >> >> > new file mode 100644
> >> >> >> > index 000..f47cdda
> >> >> >> > --- /dev/null
> >> >> >> > +++ b/gcc/testsuite/gcc.target/mips/call-clobbered-2.c
> >> >> >> > @@ -0,0 +1,21 @@
> >> >> >> > +/* Check that we handle call-clobbered FPRs correctly.  */
> >> >> >> > +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { ""
> }
> >> } */
> >> >> >> > +/* { dg-options "-mabi=32 -modd-spreg -mfp32 -ffixed-f0 -
> >> ffixed-f1 -
> >> >> >> ffixed-f2 -ffixed-f3 -ffixed-f4 -ffixed-f5 -ffixed-f6 -ffixed-
> f7 -
> >> >> ffixed-f8
> >> >> >> -ffixed-f9 -ffixed-f10 -ffixed-f11 -ffixed-f12 -ffixed-f13 -
> >> ffixed-f14 -
> >> >> >> ffixed-f15 -ffixed-f16 -ffixed-f17 -ffixed-f18 -ffixed-f19 -
> >> ffixed-f20 -
> >> >> >> ffixed-f22 -ffixed-f24 -ffixed-f26 -ffixed-f28 -ffixed-f30" }
> */
> >> >> >> > +
> >> >> >> > +void bar (void);
> >> >> >> > +float a;
> >> >> >> > +float
> >> >> >> > +foo ()
> >> >> >> > +{
> >> >> >> > +  float b = a + 1.0f;
> >> >> >> > +  bar();
> >> >> >> > +  return b;
> >> >> >> > +}
> >> >> >> > +/* { dg-final { scan-assembler-times "lwc1" 2 } } */
> >> >> >> > +/* { dg-final { scan-assembler-not "swc1" } } */
> >> >> >> > +/* { dg-final { scan-assembler-times "sdc1" 2 } } */
> >> >> >> > +/* { dg-final { scan-assembler-not "mtc" } } */
> >> >> >> > +/* { dg-final { scan-assembler-not "mfc" } } */
> >> >> >> > +/* { dg-final { scan-assembler-not "mthc" } } */
> >> >> >> > +/* { dg-final { scan-assembler-not "mfhc" } } */
> >> >> >>
> >> >> >> Why require sdc1 here?  Would Chao-Ying's patch make this use
> SWC1
> >> and
> >> >> LWC1
> >> >> >> exclusively?
> >> >> >
> >> >> > The SDC1 instructions are for callee-saved registers. I'll add
> the
> >> >> > check for two corresponding LDC1 instructions in the epilogue
> >> though
> >> >> > since I've noticed them being missing.
> >> >>
> >> >> I'm probably being dense, sorry, but why is SDC1 needed for -mfp32
> >> >> -modd-spreg?  Can't we just save and restore the odd register?
> >> >> That's technically more correct too, since we shouldn't really
> touch
> >> >> -ffixed registers.
> >> >
> >> > You are correct and I am/was aware of this...
> >> >
> >> > GCC is saving too much of the callee-saved registers when single-
> >> precision
> >> > values are live in them but this is historic behaviour. The code
> w

RE: [PATCH+1][MIPS] Implement O32 FPXX ABI (GCC)

[Matthew Fortune]

Hi Richard,

I've been working through the debug issues for O32 FPXX and O32 FP64 and have
identified some things to resolve along with proposed fixes...

What to do with debug info for double precision registers and the O32 FPXX
ABI? The choices are:
1) Do what O32 FP32 does and represent 8 byte values in two DW_OP_pieces
   referring to the relevant registers.
2) Do what O32 FP64 does and represent 8 byte values in a single register
3) Do a hybrid where 8-byte values sit in a single DW_OP_pieces.

I'm tentatively swayed to (3) as it is a slightly unusual construct that means
we should have better scope to detect it and do special handling. I've
implemented that in the patch below but am still not sure. My next choice is (1)
as it means there would only be special handling when debugging an O32 FP64
program with FPXX modules rather than both O32 FP32 and O32 FP64.

How to ensure unwinding will work when passing through functions with differing
ABI variants? There isn't much room for debate here as we simply have to
continue using the unwind table format as used in O32 FP32. This represents
32 32-bit floating point registers. A description of how O32 FP64 is modified to
cope with this is on the O32 FR wiki page:

https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking#8._Re-definition_of_the_-mabi.3D32_-mfp64_ABI

Copied here for archive
===
Changes to O32 FP64:

* 6 double-precision callee-save registers ($f20 to $f30 evens). This used to be
  12 double-precision registers.
* GNU ABI extensions for returning _Complex float and _Complex double are
  redefined to return the two components in $f0 and $f2 instead of $f0 and $f1.
* The size of registers 32->63, which represent floating-point registers, in
  dwarf unwinding tables is set to 4-byte instead of the more natural choice of
  8-byte. This is required to ensure all ABI variants which can interlink have a
  consistent view of unwind information. Unwind registers 32-63 are conceptually
  defined to represent the low and high 32-bits of a double precision value in
  even numbered registers. Register 32 is lo32($f0) and register 33 is 
hi32($f0).
  The odd-numbered 64-bit floating point registers for O32 FP64 are not
  represented in frame unwind information, this is OK because none of them are
  callee-saved. When saving or restoring a 64-bit callee-saved register two CFI
  directives must be emitted, one to represent the mapping for the lo32 frame
  register and one for the hi32 frame register.
===

I have modified the --with-fp option to be --with-fp-32 where the -32 
corresponds
to the -32 found in --with-arch-32 option. This is to support building a simple
toolchain with one 64-bit ABI and one 32-bit ABI where the FP ABI for the 32-bit
ABI can be set without affecting the 64-bit ABI.

--with-arch-64=mips3 --with-arch-32=mips2 --with-fp-32=xx

"gcc -mabi=64" then implies "-mips3 -mgp64 -mfp64"
"gcc" then implies "-mips2 -mgp32 -mfpxx"

Finally I have had to add ASM_SPECs to pass through -mhard-float, -msoft-float,
-mdouble-float, -msingle-float. Without these building ASM sources for 
soft-float
or single-float get marked as if they were hard-float double-precision with the
new binutils .MIPS.abiflags support. This does of course mean that using an old
GCC with a new binutils will lead to problems building soft-float or
single-precision code.

Attached is a patch that just addresses these issues rather than the whole FPXX
patch again. I'll merge everything together and post the whole lot when things
are resolved and more testing has been done.

Regards,
Matthew

From 4fc4f1c72e1d226691d4cebf2c5fa6de809fb409 Mon Sep 17 00:00:00 2001
From: Matthew Fortune 
Date: Sat, 31 May 2014 23:33:21 +0100
Subject: [PATCH] fpxx feature fix

---
 gcc/config.gcc |8 
 gcc/config/mips/mips.c |   34 ++
 gcc/config/mips/mips.h |   10 +-
 gcc/dwarf2cfi.c|5 +
 4 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 79268f7..dc272d8 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -3731,7 +3731,7 @@ case "${target}" in
;;
 
mips*-*-*)
-   supported_defaults="abi arch arch_32 arch_64 float fpu nan fp 
tune tune_32 tune_64 divide llsc mips-plt synci"
+   supported_defaults="abi arch arch_32 arch_64 float fpu nan 
fp_32 tune tune_32 tune_64 divide llsc mips-plt synci"
 
case ${with_float} in
"" | soft | hard)
@@ -3763,12 +3763,12 @@ case "${target}" in
;;
esac
 
-   case ${with_fp} in
+   case ${with_fp_32} in
"" | 32 | xx | 64)
# OK
;;
*)
-   echo "U

[PATCH,MIPS] Remove unused code relating to reloading fcc

[Matthew Fortune]

This is a small clean-up patch to remove code relating to reloading or moving
mips fcc registers. At some point in the past these registers were allocated
as part of register allocation but they are now statically allocated in the
backend in a round robin fashion. The code for reloading them is therefore not
necessary any more. The move costs are also irrelevant so are replaced with
a comment instead (but the cases can just be deleted if that is preferred).

Matthew

gcc/

* config/mips/mips-protos.h (mips_expand_fcc_reload): Remove.
* config/mips/mips.c (mips_expand_fcc_reload): Remove.
(mips_move_to_gpr_cost): ST_REGS can't be moved.
(mips_move_from_gpr_cost): Likewise.
(mips_register_move_cost): Likewise.

---
 gcc/config/mips/mips-protos.h |1 -
 gcc/config/mips/mips.c|   41 -
 2 files changed, 4 insertions(+), 38 deletions(-)

diff --git a/gcc/config/mips/mips-protos.h b/gcc/config/mips/mips-protos.h
index 0b8125a..0b32a70 100644
--- a/gcc/config/mips/mips-protos.h
+++ b/gcc/config/mips/mips-protos.h
@@ -232,7 +232,6 @@ extern bool mips_use_pic_fn_addr_reg_p (const_rtx);
 extern rtx mips_expand_call (enum mips_call_type, rtx, rtx, rtx, rtx, bool);
 extern void mips_split_call (rtx, rtx);
 extern bool mips_get_pic_call_symbol (rtx *, int);
-extern void mips_expand_fcc_reload (rtx, rtx, rtx);
 extern void mips_set_return_address (rtx, rtx);
 extern bool mips_move_by_pieces_p (unsigned HOST_WIDE_INT, unsigned int);
 extern bool mips_store_by_pieces_p (unsigned HOST_WIDE_INT, unsigned int);
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 73b6963..1fb5ba2 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -7195,35 +7195,6 @@ mips_function_ok_for_sibcall (tree decl, tree exp 
ATTRIBUTE_UNUSED)
   return true;
 }
 

-/* Emit code to move general operand SRC into condition-code
-   register DEST given that SCRATCH is a scratch TFmode FPR.
-   The sequence is:
-
-   FP1 = SRC
-   FP2 = 0.0f
-   DEST = FP2 < FP1
-
-   where FP1 and FP2 are single-precision FPRs taken from SCRATCH.  */
-
-void
-mips_expand_fcc_reload (rtx dest, rtx src, rtx scratch)
-{
-  rtx fp1, fp2;
-
-  /* Change the source to SFmode.  */
-  if (MEM_P (src))
-src = adjust_address (src, SFmode, 0);
-  else if (REG_P (src) || GET_CODE (src) == SUBREG)
-src = gen_rtx_REG (SFmode, true_regnum (src));
-
-  fp1 = gen_rtx_REG (SFmode, REGNO (scratch));
-  fp2 = gen_rtx_REG (SFmode, REGNO (scratch) + MAX_FPRS_PER_FMT);
-
-  mips_emit_move (copy_rtx (fp1), src);
-  mips_emit_move (copy_rtx (fp2), CONST0_RTX (SFmode));
-  emit_insn (gen_slt_sf (dest, fp2, fp1));
-}
-

 /* Implement MOVE_BY_PIECES_P.  */
 
 bool
@@ -12045,8 +12016,8 @@ mips_move_to_gpr_cost (enum machine_mode mode 
ATTRIBUTE_UNUSED,
   return 4;
 
 case ST_REGS:
-  /* LUI followed by MOVF.  */
-  return 4;
+  /* Not possible.  ST_REGS are never moved.  */
+  return 0;
 
 case COP0_REGS:
 case COP2_REGS:
@@ -12082,9 +12053,8 @@ mips_move_from_gpr_cost (enum machine_mode mode, 
reg_class_t to)
   return 4;
 
 case ST_REGS:
-  /* A secondary reload through an FPR scratch.  */
-  return (mips_register_move_cost (mode, GENERAL_REGS, FP_REGS)
- + mips_register_move_cost (mode, FP_REGS, ST_REGS));
+  /* Not possible.  ST_REGS are never moved.  */
+  return 0;
 
 case COP0_REGS:
 case COP2_REGS:
@@ -12117,9 +12087,6 @@ mips_register_move_cost (enum machine_mode mode,
   if (to == FP_REGS && mips_mode_ok_for_mov_fmt_p (mode))
/* MOV.FMT.  */
return 4;
-  if (to == ST_REGS)
-   /* The sequence generated by mips_expand_fcc_reload.  */
-   return 8;
 }
 
   /* Handle cases in which only one class deviates from the ideal.  */
-- 
1.7.1

RE: [PATCH,MIPS] Remove unused code relating to reloading fcc

[Matthew Fortune]

Richard Sandiford  writes:
> Matthew Fortune  writes:
> > This is a small clean-up patch to remove code relating to reloading or
> moving
> > mips fcc registers. At some point in the past these registers were
> allocated
> > as part of register allocation but they are now statically allocated in
> the
> > backend in a round robin fashion. The code for reloading them is therefore
> not
> > necessary any more. The move costs are also irrelevant so are replaced
> with
> > a comment instead (but the cases can just be deleted if that is
> preferred).
> 
> I think removing the cases would be better.
> 
> OK with that change.  Thanks for cleaning this up.

Re-posting as I missed removing the ST_REGS handling code from
mips_secondary_reload_class.

Is this still OK? Testsuite run on mips-unknown-linux-gnu shows no change
in pass/fail.

Regards,
Matthew

gcc/

* config/mips/mips-protos.h (mips_expand_fcc_reload): Remove.
* config/mips/mips.c (mips_expand_fcc_reload): Remove.
(mips_move_to_gpr_cost): Remove ST_REGS case.
(mips_move_from_gpr_cost): Likewise.
(mips_register_move_cost): Likewise.
(mips_secondary_reload_class): Likewise.

diff --git a/gcc/config/mips/mips-protos.h b/gcc/config/mips/mips-protos.h
index 0b8125a..0b32a70 100644
--- a/gcc/config/mips/mips-protos.h
+++ b/gcc/config/mips/mips-protos.h
@@ -232,7 +232,6 @@ extern bool mips_use_pic_fn_addr_reg_p (const_rtx);
 extern rtx mips_expand_call (enum mips_call_type, rtx, rtx, rtx, rtx, bool);
 extern void mips_split_call (rtx, rtx);
 extern bool mips_get_pic_call_symbol (rtx *, int);
-extern void mips_expand_fcc_reload (rtx, rtx, rtx);
 extern void mips_set_return_address (rtx, rtx);
 extern bool mips_move_by_pieces_p (unsigned HOST_WIDE_INT, unsigned int);
 extern bool mips_store_by_pieces_p (unsigned HOST_WIDE_INT, unsigned int);
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 585b755..cff1d38 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -7195,35 +7195,6 @@ mips_function_ok_for_sibcall (tree decl, tree exp 
ATTRIBUTE_UNUSED)
   return true;
 }
 

-/* Emit code to move general operand SRC into condition-code
-   register DEST given that SCRATCH is a scratch TFmode FPR.
-   The sequence is:
-
-   FP1 = SRC
-   FP2 = 0.0f
-   DEST = FP2 < FP1
-
-   where FP1 and FP2 are single-precision FPRs taken from SCRATCH.  */
-
-void
-mips_expand_fcc_reload (rtx dest, rtx src, rtx scratch)
-{
-  rtx fp1, fp2;
-
-  /* Change the source to SFmode.  */
-  if (MEM_P (src))
-src = adjust_address (src, SFmode, 0);
-  else if (REG_P (src) || GET_CODE (src) == SUBREG)
-src = gen_rtx_REG (SFmode, true_regnum (src));
-
-  fp1 = gen_rtx_REG (SFmode, REGNO (scratch));
-  fp2 = gen_rtx_REG (SFmode, REGNO (scratch) + MAX_FPRS_PER_FMT);
-
-  mips_emit_move (copy_rtx (fp1), src);
-  mips_emit_move (copy_rtx (fp2), CONST0_RTX (SFmode));
-  emit_insn (gen_slt_sf (dest, fp2, fp1));
-}
-

 /* Implement MOVE_BY_PIECES_P.  */
 
 bool
@@ -12044,10 +12015,6 @@ mips_move_to_gpr_cost (enum machine_mode mode 
ATTRIBUTE_UNUSED,
   /* MFC1, etc.  */
   return 4;
 
-case ST_REGS:
-  /* LUI followed by MOVF.  */
-  return 4;
-
 case COP0_REGS:
 case COP2_REGS:
 case COP3_REGS:
@@ -12081,11 +12048,6 @@ mips_move_from_gpr_cost (enum machine_mode mode, 
reg_class_t to)
   /* MTC1, etc.  */
   return 4;
 
-case ST_REGS:
-  /* A secondary reload through an FPR scratch.  */
-  return (mips_register_move_cost (mode, GENERAL_REGS, FP_REGS)
- + mips_register_move_cost (mode, FP_REGS, ST_REGS));
-
 case COP0_REGS:
 case COP2_REGS:
 case COP3_REGS:
@@ -12117,9 +12079,6 @@ mips_register_move_cost (enum machine_mode mode,
   if (to == FP_REGS && mips_mode_ok_for_mov_fmt_p (mode))
/* MOV.FMT.  */
return 4;
-  if (to == ST_REGS)
-   /* The sequence generated by mips_expand_fcc_reload.  */
-   return 8;
 }
 
   /* Handle cases in which only one class deviates from the ideal.  */
@@ -12184,23 +12143,6 @@ mips_secondary_reload_class (enum reg_class rclass,
   if (ACC_REG_P (regno))
 return reg_class_subset_p (rclass, GR_REGS) ? NO_REGS : GR_REGS;
 
-  /* We can only copy a value to a condition code register from a
- floating-point register, and even then we require a scratch
- floating-point register.  We can only copy a value out of a
- condition-code register into a general register.  */
-  if (reg_class_subset_p (rclass, ST_REGS))
-{
-  if (in_p)
-   return FP_REGS;
-  return GP_REG_P (regno) ? NO_REGS : GR_REGS;
-}
-  if (ST_REG_P (regno))
-{
-  if (!in_p)
-   return FP_REGS;
-  return reg_class_subset_p (rclass, GR_REGS) ? NO_REGS : GR_REGS;
-}
-
   if (reg_class_subset_p (rclass, FP_REGS))
 {
   if (MEM_P (x)

RE: RFA: Make LRA temporarily eliminate addresses before testing constraints

[Matthew Fortune]

> On 2014-06-16, 12:12 PM, Robert Suchanek wrote:
> > Pinging for approval.
> >
> > This part of the patch will be needed for MIPS16. The second part to
> enable
> > LRA in MIPS has been already approved.
> >
> 
>Sorry, Robert.  I thought you are waiting for some Richard's comment
> (actually he knows the code well and wrote address decoding in rtlanal.c).
> 
>The patch is ok for me and makes LRA even more portable as it adds a
> new profitable address transformation and the code can be useful for
> other targets too.

Thanks.

Core LRA change committed as: r211802
MIPS LRA committed as: r211805

Matthew

RE: [PATCH,MIPS] MIPS64r6 support

[Matthew Fortune]

Richard Sandiford  writes:
> Sorry for the slow review.

And my slow response :-)

> Matthew Fortune  writes:
> > The initial support for MIP64r6 is intentionally minimal to make
> review
> > easier. Performance enhancements and use of new MIPS64r6 features will
> > be introduced separately. The current patch makes no attempt to
> > get the testsuite appropriately configured for MIPS64r6 as the
> existing
> > structure relies on each MIPS revision being a superset of the
> previous.
> > Recommendations on how to rework the mips.exp logic to cope with this
> > would be appreciated.
> 
> Could you give an example of the kind of thing you mean?

You have actually covered the cases I was concerned about below. The
problem cases are those tests that already have a isa/isa_rev >= ...
 
> If tests really do need r5 or earlier, we should enforce that in the
> dg-options.  E.g. for conditional moves we should add an isa_rev
> limit to the existing tests and add new ones with isa_rev>=6.

OK. Steve has actually been working on this in parallel to the
review and has taken this approach.
 
> I suppose we'll need a way of specifying an isa_rev range, say
> "isa_rev=2-5".  That should be a fairly localised change though.

There appear to be about 9 tests that are not fixed by educating mips.exp
about flags which are not supported on R6. Steve has initially dealt with
these via forbid_cpu=mips.*r6 but I guess it would be cleaner to try and
support an isa_rev range. I'll see we can club together enough tcl skills
to write it :-)

> Maybe just change the comment to:
> 
>   "An address suitable for a @code{prefetch} instruction, or for any
> other
> instruction with the same addressing mode as @code{prefetch}."
> 
> perhaps going on to say what the microMIPS, r6 and other cases are,
> if you think that's better.
> 
> You need to update md.texi too; it isn't "yet" automatic.

Thanks. I've omitted the detail about what the cases are as the code
speaks for itself.

> > (if_then_else (match_test "TARGET_MICROMIPS")
> >  (match_test "umips_12bit_offset_address_p (op, mode)")
> > -(match_test "mips_address_insns (op, mode, false)")))
> > +(if_then_else (match_test "ISA_HAS_PREFETCH_9BIT")
> > +   (match_test "mipsr6_9bit_offset_address_p (op, mode)")
> > +   (match_test "mips_address_insns (op, mode, false)"
>
> Please use (cond ...) instead.

It seems I cannot use cond in a predicate expression, so I've had to
leave it as is.

> > diff --git a/gcc/config/mips/linux.h b/gcc/config/mips/linux.h
> > index e539422..751623f 100644
> > --- a/gcc/config/mips/linux.h
> > +++ b/gcc/config/mips/linux.h
> > @@ -18,8 +18,9 @@ along with GCC; see the file COPYING3.  If not see
> >  <http://www.gnu.org/licenses/>.  */
> >
> >  #define GLIBC_DYNAMIC_LINKER \
> > -  "%{mnan=2008:/lib/ld-linux-mipsn8.so.1;:/lib/ld.so.1}"
> > +  "%{mnan=2008|mips32r6|mips64r6:/lib/ld-linux-
> mipsn8.so.1;:/lib/ld.so.1}"
> >
> >  #undef UCLIBC_DYNAMIC_LINKER
> >  #define UCLIBC_DYNAMIC_LINKER \
> > -  "%{mnan=2008:/lib/ld-uClibc-mipsn8.so.0;:/lib/ld-uClibc.so.0}"
> > +  "%{mnan=2008|mips32r6|mips64r6:/lib/ld-uClibc-mipsn8.so.0;" \
> > +  ":/lib/ld-uClibc.so.0}"
> 
> Rather than update all the specs like this, I think we should force
> -mnan=2008 onto the command line for r6 using DRIVER_SELF_SPECS.
> See e.g. MIPS_ISA_SYNCI_SPEC.

I agree this could be simpler and your comment has made me realise the
implementation in the patch is wrong for configurations like
mipsisa32r6-unknown-linux-gnu. The issue for both the current patch and
your suggestion is that they rely on MIPS_ISA_LEVEL_SPEC having been
applied but this only happens in the vendor triplets. The --with-arch*
options used with mips-unknown-linux-gnu would be fine as they place
an arch option on the command line.

If I add MIPS_ISA_LEVEL_SPEC to the DRIVER_SELF_SPECS generic
definition in mips.h then I believe that would fix the problem. Any new
spec I add for R6/nan setting would also need adding to the generic
DRIVER_SELF_SPECS in mips.h and any vendor definitions of
DRIVER_SELF_SPECS.

> > diff --git a/gcc/config/mips/mips-protos.h b/gcc/config/mips/mips-
> protos.h
> > index 0b32a70..9560506 100644
> > --- a/gcc/config/mips/mips-protos.h
> > +++ b/gcc/config/mips/mips-protos.h 
> > @@ -4186,6 +4230,46 @@ mips_rtx_costs (rtx x, int code, int
> outer_code, int opno ATTRIBUTE_UNUSED,
> > }
> >*total = mips_zero_extend_cost (mode

RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute

[Matthew Fortune]

> This patch fixes an internal compiler error when micromips/nomicromips
> attributes are used.
> 
> The problem here was that the cached boolean attributes for the current
> target did not agree with the uncached attributes throwing an assertion
> error.
> 
> It appears that saving and restoring the state for micromips was
> missing, just like there is for mips16. OK to apply?

Just to check, the reason we don't see this in the current testsuite is that
there is no test with both MIPS and microMIPS functions?

> gcc/
>   * config/mips/mips.c (micromips_globals): New variable.
>   (mips_set_compression_mode): Save and reinitialize target-dependent
>   state for microMIPS.
> 
> gcc/testsuite
>   * gcc.target/mips/umips-attr.c: New test.

OK.

Matthew


> ---
>  gcc/config/mips/mips.c | 10 ++
>  gcc/testsuite/gcc.target/mips/umips-attr.c | 13 +
>  2 files changed, 23 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/mips/umips-attr.c
> 
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> bf69850..959a672 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -676,6 +676,9 @@ const char *mips_hi_relocs[NUM_SYMBOL_TYPES];
>  /* Target state for MIPS16.  */
>  struct target_globals *mips16_globals;
> 
> +/* Target state for MICROMIPS.  */
> +struct target_globals *micromips_globals;
> +
>  /* Cached value of can_issue_more. This is cached in
> mips_variable_issue hook
> and returned from mips_sched_reorder2.  */  static int
> cached_can_issue_more; @@ -17182,6 +17185,13 @@
> mips_set_compression_mode (unsigned int compression_mode)
>else
>   restore_target_globals (mips16_globals);
>  }
> +  else if (compression_mode & MASK_MICROMIPS)
> +{
> +  if (!micromips_globals)
> + micromips_globals = save_target_globals_default_opts ();
> +  else
> + restore_target_globals (micromips_globals);
> +}
>else
>  restore_target_globals (&default_target_globals);
> 
> diff --git a/gcc/testsuite/gcc.target/mips/umips-attr.c
> b/gcc/testsuite/gcc.target/mips/umips-attr.c
> new file mode 100644
> index 000..f8c4517
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/umips-attr.c
> @@ -0,0 +1,13 @@
> +/* { dg-options "(-mmicromips)" } */
> +
> +int MICROMIPS
> +foo (int a)
> +{
> +  return a;
> +}
> +
> +int NOMICROMIPS
> +foo2 (int a)
> +{
> +  return a;
> +}
> --
> 2.2.2
> 

RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute

[Matthew Fortune]

> >  We could add -mflip-micromips complementing -mflip-mips16 and use
> > that for testing too.  Chances are it'd reveal further issues.
> > Looking at how
> > -mflip-mips16 has been implemented it does not appear to me adding
> > -mflip-micromips would be a lot of effort.
> 
> I'm in favour of adding such a switch since the testsuite doesn't cover
> a mixture of MIPS and microMIPS code.

It certainly seems that we need a bit more coverage here in order that
we can mostly stick to testing one or two MIPS configurations per commit.

We'll have some MIPS machines in the compile farm shortly which may allow
us to at least do the full all-config build of the toolchain more easily
even if that doesn't extend to testing all the configs.

> 
> Regards,
> Robert
> 
> gcc/
>   * config/mips/mips.h (micromips_globals): Declare.

OK, thanks.

Matthew

RE: PING^3: [PATCH]: New configure options that make the compiler use -fPIE and -pie as default option

[Matthew Fortune]

The change for MIPS looks fine by visual inspection and I've built both
a default pie and default no-pie compiler. The default pie won't build
glibc but I am pretty sure it is not down to this patch. I haven't had
time to look into why it won't build though, something related to
selecting the CRT files.

Thanks,
Matthew

RE: [Patch MIPS] Enable TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook

[Matthew Fortune]

Hi Robert,

> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> c3755f5..3c8ac30 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -19415,6 +19415,17 @@ mips_lra_p (void)  {
>return mips_lra_flag;
>  }
> +
> +/* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS.  */
> +
> +static reg_class_t
> +mips_ira_change_pseudo_allocno_class (int regno, reg_class_t
> +allocno_class) {
> +  if (FLOAT_MODE_P (PSEUDO_REGNO_MODE (regno)) || allocno_class !=
> ALL_REGS)
> +return allocno_class;
> +  return GR_REGS;
> +}
> +

I'm concerned that this may not be the right condition but either way,
I think it is better to switch this around to have the special case
as the conditional. I found it difficult to understand what it is
doing even when I know the intent :-) A comment about the purpose seems
appropriate too here as it won't be obvious to someone new.

Aren't there some fixed point modes that should go in FPRs too? I guess
paired single (v2sf) doesn't need mentioning as it would never be
allowed in GR_REGS so pseudos of that mode would never get ALL_REGS,
is that correct? I.e. will we only see ALL_REGS if a particular
pseudo/mode truly can be placed in any register according to the
hard_regno_ok rules?

Thanks,
Matthew

> 
> 
>  /* Initialize the GCC target structure.  */  #undef
> TARGET_ASM_ALIGNED_HI_OP @@ -19671,6 +19682,8 @@ mips_lra_p (void)
> #define TARGET_SPILL_CLASS mips_spill_class  #undef TARGET_LRA_P
> #define TARGET_LRA_P mips_lra_p
> +#undef TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
> +#define TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
> +mips_ira_change_pseudo_allocno_class
> 
>  struct gcc_target targetm = TARGET_INITIALIZER;
> 
> 
> diff --git a/gcc/testsuite/gcc.target/mips/pr65862-1.c
> b/gcc/testsuite/gcc.target/mips/pr65862-1.c
> new file mode 100644
> index 000..0c00092
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/pr65862-1.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> +/* { dg-final { scan-assembler-not "\\\$f\[0-9\]+" } } */ int a, c; int
> +*b, *d; void fn1(int p1, int *p2(void *, void *), void *p3(void *, void
> +*, int)) {
> +  int n = c;
> +  for (;;) {
> +a = 1;
> +for (; a < n;) {
> +  *d = p1 && p2(0, (int *) ((long)p1 + 1));
> +  p3(0, b + p1, 0);
> +}
> +  }
> +}
> diff --git a/gcc/testsuite/gcc.target/mips/pr65862-2.c
> b/gcc/testsuite/gcc.target/mips/pr65862-2.c
> new file mode 100644
> index 000..c6a2641
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/pr65862-2.c
> @@ -0,0 +1,31 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> +/* { dg-final { scan-assembler-not "\\\$f\[0-9\]+" } } */ int a, b, d,
> +e, j, k, n, o; unsigned c, h, i, l, m, p; int *f; int *g; int fn1(int
> +p1) { return p1 - a; }
> +
> +int fn2() {
> +  b = b + 1 - a;
> +  e = 1 + o + 1518500249;
> +  d = d + n;
> +  c = (int)c + g[0];
> +  b = b + m + 1;
> +  d = d + p + 1518500249;
> +  d = d + k - 1;
> +  c = fn1(c + j + 1518500249);
> +  e = fn1(e + i + 1);
> +  d = d + h + 1859775393 - a;
> +  c = fn1(c + (d ^ 1 ^ b) + g[1] + 1);
> +  b = fn1(b + m + 3);
> +  d = fn1(d + l + 1);
> +  b = b + (c ^ 1) + p + 1;
> +  e = fn1(e + (b ^ c ^ d) + n + 1);
> +  d = o;
> +  b = 0;
> +  e = e + k + 1859775393;
> +  f[0] = e;
> +  return a;
> +}
> --
> 2.2.2

RE: [Patch MIPS] Enable TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook

[Matthew Fortune]

Robert Suchanek  writes:
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index c3755f5..976f844 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -19415,6 +19415,21 @@ mips_lra_p (void)
>  {
>return mips_lra_flag;
>  }
> +
> +/* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS.  */
> +
> +static reg_class_t
> +mips_ira_change_pseudo_allocno_class (int regno, reg_class_t
> allocno_class)
> +{
> +  /* LRA will generate unnecessary reloads because the LRA's cost pass
> finds
> + cheaper to move data to/from memory into FP regs rather than GP
> regs.
> + By narrowing the class for allocnos to GR_REGS for integral modes
> early,
> + we refrain from using FP regs until they are absolutely necessary.
> */

I'm not sure this comment is accurately describing the issue (or I have
misunderstood something). I thought this change is to counter LRA's
tendency to use an FPR as a spill instead of memory?

i.e.
/* LRA will allocate an FPR for an integer mode pseudo instead of spilling
   to memory if an FPR is present in the allocno class.  It is rare that
   we actually need to place an integer mode value in an FPR so where
   possible limit the allocation to GR_REGS.  This will slightly pessimize
   code that involves integer to/from float conversions as these will have
   to reload into FPRs in LRA. Such reloads are sometimes eliminated and
   sometimes only partially eliminated.  We choose to take this penalty
   in order to eliminate usage of FPRs in code that does not use floating
   point data.

   This change has a similar effect to increasing the cost of FPR->GPR
   register moves for integer modes so that they are higher than the cost
   of memory but changing the allocno class is more reliable.

   This is also similar to forbidding integer mode values in FPRs entirely
   but this would lead to an inconsistency in the integer to/from float
   instructions that say integer mode values must be placed in FPRs.  */

I'm keen to get the description of this right so please feel free to change
it further if it isn't clear (or correct).

I don't know if this change will lead to classic reload being unusable for
MIPS. I'm not worried about that but I think it is probably wise to
remove classic reload support for MIPS now; we are dependent on LRA for
several features already.

Do you have any details on when we are left with suboptimal code for
int->float conversions? I'd like to keep a record of them in this thread
or in the comment so we know what is left to fix.

> +  if (INTEGRAL_MODE_P (PSEUDO_REGNO_MODE (regno)) && allocno_class ==
> ALL_REGS)
> +return GR_REGS;
> +  return allocno_class;
> +}
> +

Trim the extra trailing newline.

OK to commit if you are happy with the comment.

Thanks,
Matthew

> 
> 
>  /* Initialize the GCC target structure.  */
>  #undef TARGET_ASM_ALIGNED_HI_OP
> @@ -19671,6 +19686,8 @@ mips_lra_p (void)
>  #define TARGET_SPILL_CLASS mips_spill_class
>  #undef TARGET_LRA_P
>  #define TARGET_LRA_P mips_lra_p
> +#undef TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
> +#define TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
> mips_ira_change_pseudo_allocno_class
> 
>  struct gcc_target targetm = TARGET_INITIALIZER;
> 

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

[Matthew Fortune]

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 add a comment saying that this case does not get bonded by
any known hardware even though it could be valid to bond them if it
is the second load that clobbers the base.

>+  /* Loading in same registers.  */
>+  if (load_p
>+  && REGNO (reg1) == REGNO (reg2))
>+return false;
>+
>+  /* The loads/stores are not of same type.  */
>+  rc1 = REGNO_REG_CLASS (REGNO (reg1));
>+  rc2 = REGNO_REG_CLASS (REGNO (reg2));
>+  if (rc1 != rc2
>+  && !reg_class_subset_p (rc1, rc2)
>+  && !reg_class_subset_p (rc2, rc1))
>+return false;
>+
>+  if (abs (offset1 - offset2) != GET_MODE_SIZE (mode))
>+return false;
>+
>+  return true;
>+}
>+
> /* OPERANDS describes the operands to a pair of SETs, in the order
>dest1, src1, dest2, src2.  Return true if the operands can be used
>in an LWP or SWP instruction; LOAD_P says which.  */
>diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
>index ec69ed5..1bd0dae 100644
>--- a/gcc/config/mips/mips.h
>+++ b/gcc/config/mips/mips.h
>@@ -3147,3 +3147,7 @@ extern GTY(()) struct target_globals *mips16_globals;
> #define STANDARD_STARTFILE_PREFIX_1 "/lib64/"
> #define STANDARD_STARTFILE_PREFIX_2 "/usr/lib64/"
> #endif
>+
>+#define ENABLE_LD_ST_PAIRS \
>+  (TARGET_LOAD_STORE_PAIRS && TUNE_P5600 \
>+   && !TARGET_MICROMIPS && !TARGET_FIX_24K)

I've already forgotten

RE: [PATCH 6/13] mips musl support

[Matthew Fortune]

Szabolcs Nagy  writes:
> Set up dynamic linker name for mips.
> 
> gcc/Changelog:
> 
> 2015-04-16  Gregor Richards  
> 
>   * config/mips/linux.h (MUSL_DYNAMIC_LINKER): Define.

I understand that mips musl is o32 only currently is that correct?
There does however appear to be both soft and hard float variants
listed in the musl docs. Do you plan on using the same dynamic linker
name for both float variants? No problem if so but someone must have
decided to have unique names for big and little endian so I thought
it worth checking.

Also, are you aware of the two nan encoding formats that MIPS has
and the support present in glibc's dynamic linker to deal with it?

I wonder if it would be wise to refuse to target musl unless the
ABI is known to be supported so that we can avoid compatibility
issues when different ABI variants are added in musl.

Thanks,
Matthew

RE: [PATCH 6/13] mips musl support

[Matthew Fortune]

Rich Felker  writes:
> On Tue, Apr 21, 2015 at 01:58:02PM +0000, Matthew Fortune wrote:
> > Szabolcs Nagy  writes:
> > > Set up dynamic linker name for mips.
> > >
> > > gcc/Changelog:
> > >
> > > 2015-04-16  Gregor Richards  
> > >
> > >   * config/mips/linux.h (MUSL_DYNAMIC_LINKER): Define.
> >
> > I understand that mips musl is o32 only currently is that correct?
> 
> This is correct. Other ABIs if/when we support them will have different
> names.
> 
> > There does however appear to be both soft and hard float variants
> > listed in the musl docs. Do you plan on using the same dynamic linker
> > name for both float variants? No problem if so but someone must have
> > decided to have unique names for big and little endian so I thought
> it
> > worth checking.
> 
> No, it's supposed to be different (-sf suffix for soft float; see
> arch/mips/reloc.h in musl source). If this didn't make it into the
> patches it's an omission, probably because we didn't officially support
> the sf ABI at all for a long time.
> 
> > Also, are you aware of the two nan encoding formats that MIPS has and
> > the support present in glibc's dynamic linker to deal with it?
> 
> I am aware but somewhat skeptical of treating it as yet another
> dimension to ABI and the resulting ABI combinatorics. The vast majority
> of programs couldn't care less which is which and whether a NAN is
> quiet or signaling. Officially we just use the classic mips ABI (with
> qnan/snan swapped vs other archs) but there's no harm in somebody doing
> the opposite if they really know what they're doing.

Couldn't agree more here but I know some people have been concerned about
it so the strict rules were put in place. I will attempt to remember and
copy the musl list when putting out a plan for formally relaxing the nan
encoding rules. The proposal is probably less than 2 weeks away from being
ready to review, it does of course make certain assumptions originating
from glibc as reference but is an independent ABI proposal.
 
> > I wonder if it would be wise to refuse to target musl unless the ABI
> > is known to be supported so that we can avoid compatibility issues
> > when different ABI variants are added in musl.
> 
> Possibly, though this might make bootstrapping new ABIs harder.

Indeed. The other alternative would be to set the dynamic linker name
to something slightly silly for unsupported ABIs like /lib/fixme.so
which would make it possible to bootstrap via the addition of a symlink
but it is clearly not the approved name.

thanks,
Matthew

RE: [Patch, MIPS] Minor cleanup in mips.md

[Matthew Fortune]

> 2015-04-23  Steve Ellcey  
> 
>   * config/mips/mips.md: (*madd4) Remove accum_in attribute.
>   (*madd3): Ditto.
>   (*msub4): Ditto.
>   (*msub3): Ditto.
>   (*nmadd4): Ditto.
>   (*nmadd3): Ditto.
>   (*nmadd4_fastmath): Ditto.
>   (*nmadd3_fastmath): Ditto.
>   (*nmsub4): Ditto.
>   (*nmsub3): Ditto.
>   (*nmsub4_fastmath): Ditto.
>   (*nmsub3_fastmath): Ditto.

OK, thanks.

Matthew

RE: [PATCH] PR 62173, re-shuffle insns for RTL loop invariant hoisting

[Matthew Fortune]

> Hi Matthew,
> 
> 2015-04-21 15:24 GMT+01:00 Jiong Wang :
> 
> >
> > 2015-04-21  Jiong Wang  
> >
> > gcc/
> >   * loop-invariant.c (find_defs): Enable DF_DU_CHAIN build.
> >   (vfp_const_iv): New hash table.
> >   (expensive_addr_check_p): New boolean.
> >   (init_inv_motion_data): Initialize new variables.>
> >   (free_inv_motion_data): Release hash table.
> >   (create_new_invariant): Set cheap_address to false for iv in
> >   vfp_const_iv table.
> >   (find_invariant_insn): Skip dependencies check for iv in
> vfp_const_iv
> >   table.
> >   (use_for_single_du): New function.
> >   (reshuffle_insn_with_vfp): Likewise.
> >   (find_invariants_bb): Call reshuffle_insn_with_vfp.
> 
> Is it possible for you to test this patch on spec2k6 on MIPS?
> especially 436.cactusADM.
> I am interested in the performance impact on other load/store machines.

I will certainly try and fit it in. It may take me a week to get back to
you though as I'm away on vacation for a few days.

Matthew 

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

[Matthew Fortune]

Steve Ellcey   writes:
> This patch changes the default processor for mips4 from the r8000 to
> the
> r1.  There are several reasons for this change, the main one
> being the difference in the r8000 madd instruction and the rest of the
> mips4
> family.  The r8000 has a fused madd instruction (no truncation between
> the
> multiply and add) but the rest of the mips4 CPUs have unfused madd
> instructions where truncation is done and the result is identical to
> using
> separate multiply and add instructions.
> 
> I plan to submit a patch later to clean up MIPS handling of madd and
> other
> fma style instructions but to do that it is necessary to do different
> things
> for r8000 vs the other mips4 targets and that is easier if r8000 is not
> the
> default processor selected when -mips4 is used.
> 
> I have also been told that very few r8000 machines were ever released
> and
> that the r10k/r12k/etc. series is probably the most common mips4
> machines
> still out there and so would be a better default.
> 
> This patch will have no affect on compiles where a specific processor
> is
> specified (i.e. -march=r8, -march=r1), but only when using the
> more generic -mips4 or -march=mips4 flags.  The only visible difference
> in that case is that GCC will use the r10k instruction scheduler
> instead
> of the generic MIPS scheduler (there is no r8000 specific scheduler)
> and
> that should be a good thing for most mips4 machines.
> 
> Tested with the mips-linux-gnu toolchain.
> 
> OK for checkin?
> 
> Steve Ellcey
> sell...@imgtec.com
> 
> 
> 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.

Thanks,
Matthew

RE: [PATCH 6/13] mips musl support

[Matthew Fortune]

H.J. Lu  writes:
> On Mon, Apr 27, 2015 at 7:40 AM, Szabolcs Nagy 
> wrote:
> >
> >
> > On 21/04/15 15:59, Matthew Fortune wrote:
> >> Rich Felker  writes:
> >>> On Tue, Apr 21, 2015 at 01:58:02PM +, Matthew Fortune wrote:
> >>>> There does however appear to be both soft and hard float variants
> >
> > Patch v2.
> >
> > Now all the ABI variants musl plans to support are represented.
> >
> > gcc/Changelog:
> >
> > 2015-04-27  Gregor Richards  
> > Szabolcs Nagy  
> >
> > * config/mips/linux.h (MUSL_DYNAMIC_LINKER32): Define.
> > (MUSL_DYNAMIC_LINKER64, MUSL_DYNAMIC_LINKERN32): Define.
> > (GNU_USER_DYNAMIC_LINKERN32): Update.
> 
> You checked in config/linux.h CHOOSE_DYNAMIC_LINKER change without
> config/mips/linux.h change.  Now linux-mips is broken.

The MIPS patch is OK. I am concerned that you are aiming for one
dynamic linker per ABI variant in musl but are not accounting for
soft-float up front in n32/n64. There is time to reconsider this
before any of this code gets to a versioned GCC release though.

I.e. as it stands this patch is not OK for backporting to GCC 5
without further discussion.

There is also the perspective that we should be able to aim for
an ABI variant agnostic dynamic linker at some point over the next
year by working towards a build that truly uses no float and is
hence compatible with all the ABI variants.

Thanks,
Matthew

RE: [PATCH 6/13] mips musl support

[Matthew Fortune]

Szabolcs Nagy  writes:
> On 08/05/15 15:25, Matthew Fortune wrote:
> > H.J. Lu  writes:
> >> On Mon, Apr 27, 2015 at 7:40 AM, Szabolcs Nagy
> >> 
> >> wrote:
> >>>
> >>>
> >>> On 21/04/15 15:59, Matthew Fortune wrote:
> >>>> Rich Felker  writes:
> >>>>> On Tue, Apr 21, 2015 at 01:58:02PM +, Matthew Fortune wrote:
> >>>>>> There does however appear to be both soft and hard float variants
> >>>
> >>> Patch v2.
> >>>
> >>> Now all the ABI variants musl plans to support are represented.
> >>>
> >>> gcc/Changelog:
> >>>
> >>> 2015-04-27  Gregor Richards  
> >>> Szabolcs Nagy  
> >>>
> >>> * config/mips/linux.h (MUSL_DYNAMIC_LINKER32): Define.
> >>> (MUSL_DYNAMIC_LINKER64, MUSL_DYNAMIC_LINKERN32): Define.
> >>> (GNU_USER_DYNAMIC_LINKERN32): Update.
> >>
> >> You checked in config/linux.h CHOOSE_DYNAMIC_LINKER change without
> >> config/mips/linux.h change.  Now linux-mips is broken.
> >
> > The MIPS patch is OK. I am concerned that you are aiming for one
> > dynamic linker per ABI variant in musl but are not accounting for
> > soft-float up front in n32/n64. There is time to reconsider this
> > before any of this code gets to a versioned GCC release though.
> >
> 
> i thought musl would not want to support soft float variants of those
> abis, but now i think it does not hurt to add the -sf there too.
> 
> if you think that's ok, i can now submit the patch with %{msoft-float:-
> sf} added to all abi variants.

That's fine. Go ahead.

> > I.e. as it stands this patch is not OK for backporting to GCC 5
> > without further discussion.
> >
> > There is also the perspective that we should be able to aim for an ABI
> > variant agnostic dynamic linker at some point over the next year by
> > working towards a build that truly uses no float and is hence
> > compatible with all the ABI variants.
> 
> i'm not sure what you mean by 'a build that truly uses no float'
> 
> i thought the direction is to have a potentially hard float abi with
> kernel emulation when the fpu is not present.

With MIPS having such a rich matrix of ABI variants the need to build code
to target all variants is quite costly. We currently have to do this
regardless of whether the code in it is affected by the differences. We are
looking at ways to create more generic objects so that some libraries can
get away with fewer build variations. The major variation for MIPS is the
set of floating-point extensions so knowing that a module has no interest
in floating point code is quite valuable.

Since Rich has just pointed out that the dynamic linker and C library are
one and the same for musl then this will not be of as much value to musl.

Thanks,
Matthew

> 
> >
> > Thanks,
> > Matthew
> >

RE: [PATCH 6/13] mips musl support

[Matthew Fortune]

Jeff Law  writes:
> On 05/08/2015 10:50 AM, Joseph Myers wrote:
> >
> > Note that however the dynamic linker does properly need to save and
> > restore call-clobbered registers used for argument passing (because of
> > IFUNCs, user-provided malloc, audit hooks etc. that might affect them
> > even if the dynamic linker itself doesn't); see
> > .  So any
> > floating-point-agnostic dynamic linker would, if fixing the bugs
> > around not saving / restoring such registers, need to have
> > runtime-conditional code to save and restore them rather than simple
> > compile-time conditionals.
> Right.  So there has to be some reasonable cost way to find out if you
> have FPU registers at runtime, then select between the code paths at
> runtime.

Indeed and I think there are many ways to achieve that without hardware
support as the cost of loading a global variable in the dynamic linker
wouldn't be significant. I think MIPS ABIs are the only ones with enough
information available at runtime to achieve this currently but the
principles should apply to any arch.

The work we are investigating as part of PR65862 will help all this
as the intention is to avoid FPR usage unless floating point is actually
used. There is then some thinking involved in figuring out what no-float
should really mean: i.e. no floating point types used, no FPU insns used
and/or no floating point in any of the calls/global functions. I haven't
spent enough time thinking about which ones are the most important from
above yet.

Matthew

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

[Matthew Fortune]

Hi Sameera,

Sameera Deshpande  writes: 
> 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.
> 
> gcc/testsuite/
>  * gcc.target/mips/p5600-bonding.c : New testcase to test
> bonding.

Just 'New file.' is fine for the changelog.

>diff --git a/gcc/testsuite/gcc.target/mips/p5600-bonding.c 
>b/gcc/testsuite/gcc.target/mips/p5600-bonding.c
>new file mode 100644
>index 000..122b9f8
>--- /dev/null
>+++ b/gcc/testsuite/gcc.target/mips/p5600-bonding.c
>@@ -0,0 +1,19 @@
>+/* { dg-do compile } */
>+/* { dg-options "-dp -mtune=p5600  -mno-micromips -mno-mips16" } */
>+/* { dg-skip-if "Bonding needs peephole optimization." { *-*-* } { "-O0" 
>"-O1" } { "" } } */
>+typedef int VINT32 __attribute__ ((vector_size((16;
>+
>+void memory_operation_fun2_si(void * __restrict src, void * __restrict dest, 
>int num)

Code style applies for testcases too, return type on line above, space
after function name, line length.

>+{
>+VINT32 *vsrc = (VINT32 *)src;

Indentation.

>+VINT32 *vdest = (VINT32 *)dest;
>+int i;
>+
>+for (i = 0; i < num - 1; i+=2)
>+{

Indentation

>+  vdest[i] = (vdest[i] + vsrc[i]);

Unnecessary brackets.

>+  vdest[i + 1] = vdest[i + 1] + vsrc[i + 1];
>+}
>+}
>+/* { dg-final { scan-assembler "join2_" } }  */
>+

OK with those changes.

Thanks,
Matthew

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

[Matthew Fortune]

Jeff Law  writes:
> On 05/11/2015 01:46 PM, Jeff Law wrote:
> > On 05/11/2015 01:44 PM, Steve Ellcey wrote:
> >> On Mon, 2015-05-11 at 13:22 -0500, Segher Boessenkool wrote:
> >>> Hi Steve,
> >>>
> >>> On Mon, May 11, 2015 at 10:50:02AM -0700, Steve Ellcey wrote:
>  This patch broke a number of MIPS tests, specifically mips32r6
>  tests that look for the lsa instruction (load scaled address) which
>  shifts one register and then adds it to a second register.  I am
>  not sure if this needs to be addressed in combine.c or if we need
>  to add a peephole optimization to mips.md to handle the new
>  instruction sequence.
>  What do
>  you think?  Is the change here what you would expect to see from
>  your patch?
> >>>
> >>> Yes, this is as expected.  AFAICS the only change you need in the
> >>> MIPS backend is to change the "lsa" pattern to match a shift
> >>> instead of a mult (and change the "const_immlsa_operand" predicate
> >>> to just match 1..4 instead of the powers).
> >>>
> >>>
> >>> Segher
> >>
> >> Hm, I thought it was going to be more complicated than that, but it
> >> seems to be working.  I will do a complete test run and then submit a
> >> patch.
> > Yea, it really should be that easy.
> >
> > I'm pretty sure the sh[123]add insns in the PA need to be updated in a
> > similar manner.
> Oh, and just to be clear, I'm not expecting you to do this Steve.  It'd
> be great if you did, but not required.

Does this patch effectively change the canonicalization rules? The following
Still exists in md.texi:

@item
Within address computations (i.e., inside @code{mem}), a left shift is
converted into the appropriate multiplication by a power of two.

Thanks,
Matthew

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

[Matthew Fortune]

Segher Boessenkool  writes:
> On Mon, May 11, 2015 at 08:16:41PM +0000, Matthew Fortune wrote:
> > Does this patch effectively change the canonicalization rules? The
> > following Still exists in md.texi:
> >
> > @item
> > Within address computations (i.e., inside @code{mem}), a left shift is
> > converted into the appropriate multiplication by a power of two.
> 
> No, it makes combine *follow* those rules -- this isn't inside a MEM.

Thanks, I'm being a bit slow today.

Matthew

[PATCHv2,MIPS 2/2] Add new triplets for vendor 'img'

[Matthew Fortune]

This patch adds new triplets: mips*-img-linux* and mips*-img-elf*

The purpose of these triplets is essentially to provide a clear separation
between tools which support mips32r5 and below and tools which support
mips32r6 and above.

Thanks,
Matthew

/
* configure.ac: Add mips-img-elf triplet.

gcc/

* config.gcc: Support mips*-img-linux* and mips*-img-elf*.
* config/mips/mti-linux.h: Support mips32r6 as being the default arch.
* config/mips/t-img-elf: New.
* config/mips/t-img-linux: New.

gcc/testsuite/

* gcc.target/mips/pr37362.c: Skip for mips-img-elf.
---
 configure   |  4 ++--
 configure.ac|  4 ++--
 gcc/config.gcc  | 13 +++
 gcc/config/mips/mti-linux.h |  9 +++-
 gcc/config/mips/t-img-elf   | 38 +
 gcc/config/mips/t-img-linux | 30 ++
 gcc/testsuite/gcc.target/mips/pr37362.c |  2 +-
 7 files changed, 94 insertions(+), 6 deletions(-)
 create mode 100644 gcc/config/mips/t-img-elf
 create mode 100644 gcc/config/mips/t-img-linux

diff --git a/configure b/configure
index 7213c1b..786bbcd 100755
--- a/configure
+++ b/configure
@@ -3740,7 +3740,7 @@ case "${target}" in
   microblaze*)
 noconfigdirs="$noconfigdirs gprof"
 ;;
-  mips*-sde-elf* | mips*-mti-elf*)
+  mips*-sde-elf* | mips*-mti-elf* | mips*-img-elf*)
 if test x$with_newlib = xyes; then
   noconfigdirs="$noconfigdirs gprof"
 fi
@@ -6710,7 +6710,7 @@ case "${target}" in
   spu-*-*)
 target_makefile_frag="config/mt-spu"
 ;;
-  mips*-sde-elf* | mips*-mti-elf*)
+  mips*-sde-elf* | mips*-mti-elf* | mips*-img-elf*)
 target_makefile_frag="config/mt-sde"
 ;;
   mipsisa*-*-elfoabi*)
diff --git a/configure.ac b/configure.ac
index 2c958a2..2db94f7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1114,7 +1114,7 @@ case "${target}" in
   microblaze*)
 noconfigdirs="$noconfigdirs gprof"
 ;;
-  mips*-sde-elf* | mips*-mti-elf*)
+  mips*-sde-elf* | mips*-mti-elf* | mips*-img-elf*)
 if test x$with_newlib = xyes; then
   noconfigdirs="$noconfigdirs gprof"
 fi
@@ -2383,7 +2383,7 @@ case "${target}" in
   spu-*-*)
 target_makefile_frag="config/mt-spu"
 ;;
-  mips*-sde-elf* | mips*-mti-elf*)
+  mips*-sde-elf* | mips*-mti-elf* | mips*-img-elf*)
 target_makefile_frag="config/mt-sde"
 ;;
   mipsisa*-*-elfoabi*)
diff --git a/gcc/config.gcc b/gcc/config.gcc
index 73f0e63..d346a91 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1943,6 +1943,14 @@ mips*-*-netbsd*) # NetBSD/mips, either 
endian.
tm_file="elfos.h ${tm_file} mips/elf.h netbsd.h netbsd-elf.h 
mips/netbsd.h"
extra_options="${extra_options} netbsd.opt netbsd-elf.opt"
;;
+mips*-img-linux*)
+   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 
mips/mti-linux.h"
+   extra_options="${extra_options} linux-android.opt"
+   tmake_file="${tmake_file} mips/t-img-linux"
+   tm_defines="${tm_defines} MIPS_ISA_DEFAULT=37 MIPS_ABI_DEFAULT=ABI_32"
+   gnu_ld=yes
+   gas=yes
+   ;;
 mips*-mti-linux*)
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 
mips/mti-linux.h"
extra_options="${extra_options} linux-android.opt"
@@ -2003,6 +2011,11 @@ mips*-mti-elf*)
tmake_file="mips/t-mti-elf"
tm_defines="${tm_defines} MIPS_ISA_DEFAULT=33 MIPS_ABI_DEFAULT=ABI_32"
;;
+mips*-img-elf*)
+   tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/n32-elf.h 
mips/sde.h mips/mti-elf.h"
+   tmake_file="mips/t-img-elf"
+   tm_defines="${tm_defines} MIPS_ISA_DEFAULT=37 MIPS_ABI_DEFAULT=ABI_32"
+   ;;
 mips*-sde-elf*)
tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/n32-elf.h 
mips/sde.h"
tmake_file="mips/t-sde"
diff --git a/gcc/config/mips/mti-linux.h b/gcc/config/mips/mti-linux.h
index 98d6582..5761ab7 100644
--- a/gcc/config/mips/mti-linux.h
+++ b/gcc/config/mips/mti-linux.h
@@ -19,8 +19,15 @@ along with GCC; see the file COPYING3.  If not see
 
 /* This target is a multilib target, specify the sysroot paths.  */
 #undef SYSROOT_SUFFIX_SPEC
+#if MIPS_ISA_DEFAULT == 33 /* mips32r2 is the default */
 #define SYSROOT_SUFFIX_SPEC \
-
"%{mips32:/mips32}%{mips64:/mips64}%{mips64r2:/mips64r2}%{mips16:/mips16}%{mmicromips:/micromips}%{mabi=64:/64}%{mel|EL:/el}%{msoft-float:/sof}%{mnan=2008:/nan2008}"
+
"%{mips32:/mips32}%{mips64:/mips64}%{mips64r2:/mips64r2}%{mips32r6:/mips32r6}%{mips64r6:/mips64r6}%{mips16:/mips16}%{mmicromips:/micromips}%{mabi=64:/64}%{mel|EL:/el}%{msoft-float:/sof}%{!mips32r6:%{!mips64r6:%{mnan=2008:/nan2008}}}"
+#elif MIPS_ISA_DEFAULT == 37 /* mips32r6 is the default */
+#define SYSROOT_SUFFIX_SPEC \

RE: Follow-up to PR51471

[Matthew Fortune]

Eric Botcazou  writes:
> > IIRC, fill_eager and its related friends are all speculative in some
> way
> > and aren't those precisely the ones that are causing us problems.
> Also
> > note we have backends working around this stuff in fairly blunt ways:
> 
> I'd say that the PA back-end went a bit too far here, especially if it
> marks some insns of the epilogue as frame-related.  dwarf2cfi.c has
> special code to handle delay slots (SEQUENCEs) so it's not an all-or-
> nothing game.
> 
> > Given architectural difficulties of delay slots on modern processors,
> > would it be that painful to just not allow filling slots with frame
> > insns and let dbr try to find something else or drop in a nop?  I
> > wouldn't be all that surprised if there wasn't a measurable
> > performance difference on something like a modern Sparc.
> 
> Yes, modern SPARCs have (short) branches without delay slots.  But the
> other big contender is MIPS here and the story might be different for
> it.

MIPSr6 introduces 'compact' branches which do not have delay slots.

So the issues of filling delay slots will be less important from R6
onwards. However, delay slots remain important for now.

I haven't thought about the problem much but instinctively I'd be surprised
if a blanket restriction on frame-related instructions would lead to lots
of NOPs in delay slots.

Matthew

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

[Matthew Fortune]

>  OK to apply?
> 
> 2014-11-17  Maciej W. Rozycki  
> 
>   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!

Thanks,
Matthew

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

[Matthew Fortune]

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

> gcc/
>   * config/mips/mips.c (mips_process_sync_loop): Place a nop in the
>   delay slot of the branch likely instruction.
> 
> 
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index 02268f3..6dd3728 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -12997,7 +12997,12 @@ mips_process_sync_loop (rtx_insn *insn, rtx
> *operands)
>   This will sometimes be a delayed branch; see the write code below
>   for details.  */
>mips_multi_add_insn (is_64bit_p ? "scd\t%0,%1" : "sc\t%0,%1", at, mem,
> NULL);
> -  mips_multi_add_insn ("beq%?\t%0,%.,1b", at, NULL);
> +
> +  /* We can not put the NEWVAL = $TMP3 or CMP = 1 operations in the delay
> slot
> + of the branch if it is a branch likely because they will not execute
> when
> + the atomic operation is successful, so place a NOP in there instead.
> */
> +

I found the comment hard to digest, perhaps:

/* When using branch likely (-mfix-r1), the delay slot instruction will
   be annulled on false.  The normal delay slot instructions calculate the
   overall result of the atomic operation and must not be annulled.
   Unconditionally use a NOP instead for the branch likely case.  */

> +  mips_multi_add_insn ("beq%?\t%0,%.,1b%~", at, NULL);
> 
>/* if (INSN1 != MOVE && INSN1 != LI) NEWVAL = $TMP3 [delay slot].  */
>if (insn1 != SYNC_INSN1_MOVE && insn1 != SYNC_INSN1_LI && tmp3 !=
> newval)

Matthew

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

[Matthew Fortune]

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?

Matthew