This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch] 1/2 Expand sync_sub as sync_add if predicates don't match.


David Daney wrote:
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.

Now tested on both i686-pc-linux-gnu and x86_64-pc-linux-gnu all default languages with no regressions found.


OK to commit?



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];


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]