This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch] 1/2 Expand sync_sub as sync_add if predicates don't match.
- From: David Daney <ddaney at avtrex dot com>
- To: David Daney <ddaney at avtrex dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, richard at codesourcery dot com
- Date: Tue, 21 Aug 2007 21:53:55 -0700
- Subject: Re: [Patch] 1/2 Expand sync_sub as sync_add if predicates don't match.
- References: <46C9C182.4070806@avtrex.com> <87k5rp5krr.fsf@firetop.home>
Richard Sandiford wrote:
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.)
The attached new revision of the patch does it this way now. Light
testing on MIPS shows that it has the same effect as the first version.
Which from the point of view of my mips atomic memory operations patch
is sufficient.
I was a little hesitant to do it this way the first time because it has
the potential of adversely impacting a (hypothetical) port where
sync_sub<mode> is more efficient than sync_add<mode> the only place I could think of where this would be plausible is for cases where negating the value would cause it to no longer fit in the same number of bits as the original value (-0x8000 on mips assuming there were a subiu instruction).
If deemed better than the previous version, I will fully test this. It
would however be nice to get feedback that this is really better before
exerting a lot more effort in testing.
2007-08-21 David Daney <ddaney@avtrex.com>
* optabs.c (expand_sync_operation): Use plus insn if minus
CONST_INT_P(val).
(expand_sync_fetch_operation): Ditto.
Index: optabs.c
===================================================================
--- optabs.c (revision 127356)
+++ optabs.c (working copy)
@@ -6444,7 +6444,7 @@ expand_sync_operation (rtx mem, rtx val,
case MINUS:
icode = sync_sub_optab[mode];
- if (icode == CODE_FOR_nothing)
+ if (icode == CODE_FOR_nothing || CONST_INT_P (val))
{
icode = sync_add_optab[mode];
if (icode != CODE_FOR_nothing)
@@ -6544,7 +6544,8 @@ expand_sync_fetch_operation (rtx mem, rt
case MINUS:
old_code = sync_old_sub_optab[mode];
new_code = sync_new_sub_optab[mode];
- if (old_code == CODE_FOR_nothing && new_code == CODE_FOR_nothing)
+ if ((old_code == CODE_FOR_nothing && new_code == CODE_FOR_nothing)
+ || CONST_INT_P (val))
{
old_code = sync_old_add_optab[mode];
new_code = sync_new_add_optab[mode];