From: Richard Sandiford <richard@codesourcery.com>
To: David Daney <ddaney@avtrex.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [Patch] 1/2 Expand sync_sub as sync_add if predicates don't match.
Date: Tue, 21 Aug 2007 09:02:00 -0000 [thread overview]
Message-ID: <87k5rp5krr.fsf@firetop.home> (raw)
In-Reply-To: <46C9C182.4070806@avtrex.com> (David Daney's message of "Mon\, 20 Aug 2007 09\:29\:54 -0700")
David Daney <ddaney@avtrex.com> writes:
> While implementing the atomic memory operations for MIPS, I came across
> a problem where immediate operands were forced into a register when
> sync_sub* insns were expanded. On MIPS there is no minus instruction
> for immediate operands, instead this should be expanded as plus with the
> negative value of the immediate operand.
>
> In expand_sync_operation() and expand_sync_fetch_operation() there is
> code to handle conversion to plus if the minus insn does not exits. I
> enhanced this slightly so that it also does the conversion if the
> predicate for sync_sub* does not match, but it does match for the
> corresponding sync_add*. This allows me to write sync_sub* insns with a
> register predicate and have them used for non-immediate operands, but an
> immediate operand results in conversion to sync_add*.
>
> Tested on x86_64-pc-linux-gnu all default languages both native and -m32
> with no regressions. Also tested mips64-linux C only.
>
> :ADDPATCH middle-end:
>
> OK to commit?
FWIW, with (minus ... (const_int ...)) not being canonical rtl, I think
we should always use PLUS rather than MINUS if the operand is a CONST_INT.
E.g. the first thing expand_binop does is:
/* If subtracting an integer constant, convert this into an addition of
the negated constant. */
if (binoptab == sub_optab && GET_CODE (op1) == CONST_INT)
{
op1 = negate_rtx (mode, op1);
binoptab = add_optab;
}
and I think we should have a similar thing in expand_sync_operation
and expand_sync_fetch_operation. As well as avoiding invalid
expressions, it has the advantage of folding the negation at
generation time, which I don't think the current version would.
(Sorry if that's wrong.)
Richard
next prev parent reply other threads:[~2007-08-21 8:56 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-08-20 16:46 David Daney
2007-08-20 17:06 ` [Patch] 2/2 MIPS atomic memory operations David Daney
2007-08-21 9:02 ` Richard Sandiford
2007-09-03 6:02 ` David Daney
2007-09-03 8:30 ` Richard Sandiford
2007-09-03 9:39 ` Maxim Kuvyrkov
2007-09-03 15:35 ` Richard Sandiford
2007-09-06 21:33 ` David Daney
2007-08-21 9:02 ` Richard Sandiford [this message]
2007-08-22 6:47 ` [Patch] 1/2 Expand sync_sub as sync_add if predicates don't match David Daney
2007-08-24 0:41 ` David Daney
2007-09-05 18:16 ` Ping: " David Daney
2007-09-09 11:23 ` Ping^3: use sync_add rather than sync_sub for CONST_INTs Richard Sandiford
2007-09-09 13:47 ` Richard Guenther
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=87k5rp5krr.fsf@firetop.home \
--to=richard@codesourcery.com \
--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).