public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Sandiford <rsandifo@nildram.co.uk>
To: David Daney <ddaney@avtrex.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [Patch] 2/3 MIPS support for builtin __builtin_flush_icache().
Date: Mon, 09 Jul 2007 19:07:00 -0000	[thread overview]
Message-ID: <87bqelpfgb.fsf@firetop.home> (raw)
In-Reply-To: <46913DEA.7090904@avtrex.com> (David Daney's message of "Sun, 08 	Jul 2007 12:41:30 -0700")

Thanks for all the updates.  This patch looks good.

David Daney <ddaney@avtrex.com> writes:
> INITIALIZE_TRAMPOLINE now uses the new clear_cache insn.  This means 
> that it has to add the trampoline size to the ADDR argument before 
> expanding clear_cache.  If clear_cache expands to a library call, it has 
> to subtract BEGIN from END to get the length.  Looking at the generated 
> code it appears that the RTL optimizers see that this 
> addition/subtraction pair cancel each other out, and somewhat optimal 
> code is generated.

It's good to know that the sequence is optimised.  I agree this is
the way to go.  Just a few minor comments:

> +/* ISA includes synci, jr.hb and jalr.hb */

".  */"

> +  /* Load INC with the cache line size (rdhwr INC,$1). */
> +  inc = gen_reg_rtx (SImode);

Just for the record, this will eventually need to be Pmode too,
but we'd then need two rdhwr patterns.  I'll leave that to whoever
adds MIPS64r2 support though; there's no need to change the patch here.

> +  emit_insn (gen_rdhwr (inc , const1_rtx));

"inc, const1_rtx"

> +  else if (mips_cache_flush_func && mips_cache_flush_func[0])
> +    {
> +      rtx len = gen_reg_rtx (SImode);
> +      emit_insn (gen_sub3_insn (len, operands[1], operands[0]));

This too should be Pmode, since the two input operands are Pmode.
(AIUI, it's OK to pass a Pmode value to emit_library_call, even if
the argument is SImode.)  Please make this change, since it's
trivial in this context.

OK with those three changes.  They're all trivial, so if it compiles,
there's no need to retest.

Richard

  reply	other threads:[~2007-07-09 19:03 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-01  5:01 [Patch] 1/3 Add new " David Daney
2007-07-01  5:05 ` [Patch] 2/3 MIPS support for " David Daney
2007-07-01 10:56   ` Richard Sandiford
2007-07-05  7:50     ` David Daney
2007-07-05 19:05       ` Richard Sandiford
2007-07-08 20:00         ` David Daney
2007-07-09 19:07           ` Richard Sandiford [this message]
2007-07-11  5:45             ` David Daney
2007-07-01  5:11 ` [Patch] 3/3 FFI: Use __builtin_flush_icache() to flush MIPS i-cache David Daney
2007-07-05  8:34   ` David Daney
2007-07-05 19:08     ` Richard Sandiford
2007-07-11 18:55       ` Tom Tromey
2007-07-01  8:01 ` [Patch] 1/3 Add new builtin __builtin_flush_icache() Paolo Bonzini
2007-07-01 17:51   ` Thiemo Seufer
2007-07-01 18:47   ` David Daney
2007-07-02  5:04     ` Paolo Bonzini
2007-07-03 23:55       ` Mark Mitchell
2007-07-04  7:18         ` Paolo Bonzini
2007-07-04 17:17           ` Mark Mitchell
2007-07-04 17:10         ` David Daney
2007-07-04 17:51           ` Mark Mitchell
2007-07-05  7:36             ` David Daney
2007-07-05 17:59               ` Richard Sandiford
2007-07-05 18:23                 ` David Daney
2007-07-05 19:11                   ` Richard Sandiford
2007-07-06  6:07               ` Mark Mitchell
2007-07-06  6:19                 ` David Daney
2007-07-06 16:39                   ` Mark Mitchell
2007-07-08 19:22                     ` David Daney
2007-07-09 19:04                       ` Richard Sandiford
2007-07-11  2:22                       ` Mark Mitchell
2007-07-11  4:47                         ` David Daney
2007-07-05  9:05 ` [Patch] 4/3 i386 target support for __builtin___clear_cache() David Daney
2007-07-08 20:39   ` David Daney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bqelpfgb.fsf@firetop.home \
    --to=rsandifo@nildram.co.uk \
    --cc=ddaney@avtrex.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).