On 25 June 2014 22:44, Richard Earnshaw <rearn...@arm.com> wrote:
> On 23/06/14 07:58, Zhenqiang Chen wrote:
>> Hi,
>>
>> Swapping operands in a ccmp will lead to illegal instructions. So the
>> patch disables it in simplify_while_replacing.
>>
>> The patch is separated from
>> https://gcc.gnu.org/ml/gcc-patches/2014-02/msg01407.html.
>>
>> To make it clean. The patch adds two files: ccmp.{c,h} to hold all new
>> ccmp related functions.
>>
>> OK for trunk?
>>
>> Thanks!
>> -Zhenqiang
>>
>> ChangeLog:
>> 2014-06-23  Zhenqiang Chen  <zhenqiang.c...@linaro.org>
>>
>>         * Makefile.in: Add ccmp.o
>>         * ccmp.c: New file.
>
> Do we really need a new file for one 10-line function?  Seems like
> overkill.  I think it would be better to just drop this function into
> recog.c.
>
> Also, can you explain more clearly what the problem is with swapping the
> operands?  If this can't be done, then SWAPPABLE_OPERANDS is arguably
> doing the wrong thing; and that might mean that rtx class you've applied
> to your new code is incorrect.

Thanks for the comments.

In previous tests, I got several new fails if the operands were
swapped. I will try to reproduce it and back to you.

Thanks!
-Zhenqiang

> R.
>
>>         * ccmp.h: New file.
>>         * recog.c (simplify_while_replacing): Check ccmp_insn_p.
>>
>> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
>> index 5587b75..8757a30 100644
>> --- a/gcc/Makefile.in
>> +++ b/gcc/Makefile.in
>> @@ -1169,6 +1169,7 @@ OBJS = \
>>         builtins.o \
>>         caller-save.o \
>>         calls.o \
>> +       ccmp.o \
>>         cfg.o \
>>         cfganal.o \
>>         cfgbuild.o \
>> diff --git a/gcc/ccmp.c b/gcc/ccmp.c
>> new file mode 100644
>> index 0000000..665c2a5
>> --- /dev/null
>> +++ b/gcc/ccmp.c
>> @@ -0,0 +1,62 @@
>> +/* Conditional compare related functions
>> +   Copyright (C) 2014-2014 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.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with GCC; see the file COPYING3.  If not see
>> +<http://www.gnu.org/licenses/>.  */
>> +
>> +#include "config.h"
>> +#include "system.h"
>> +#include "coretypes.h"
>> +#include "tm.h"
>> +#include "rtl.h"
>> +#include "tree.h"
>> +#include "stringpool.h"
>> +#include "regs.h"
>> +#include "expr.h"
>> +#include "optabs.h"
>> +#include "tree-iterator.h"
>> +#include "basic-block.h"
>> +#include "tree-ssa-alias.h"
>> +#include "internal-fn.h"
>> +#include "gimple-expr.h"
>> +#include "is-a.h"
>> +#include "gimple.h"
>> +#include "gimple-ssa.h"
>> +#include "tree-ssanames.h"
>> +#include "target.h"
>> +#include "common/common-target.h"
>> +#include "df.h"
>> +#include "tree-ssa-live.h"
>> +#include "tree-outof-ssa.h"
>> +#include "cfgexpand.h"
>> +#include "tree-phinodes.h"
>> +#include "ssa-iterators.h"
>> +#include "expmed.h"
>> +#include "ccmp.h"
>> +
>> +bool
>> +ccmp_insn_p (rtx object)
>> +{
>> +  rtx x = PATTERN (object);
>> +  if (targetm.gen_ccmp_first
>> +      && GET_CODE (x) == SET
>> +      && GET_CODE (XEXP (x, 1)) == COMPARE
>> +      && (GET_CODE (XEXP (XEXP (x, 1), 0)) == IOR
>> +         || GET_CODE (XEXP (XEXP (x, 1), 0)) == AND))
>> +    return true;
>> +  return false;
>> +}
>> +
>> diff --git a/gcc/ccmp.h b/gcc/ccmp.h
>> new file mode 100644
>> index 0000000..7e139aa
>> --- /dev/null
>> +++ b/gcc/ccmp.h
>> @@ -0,0 +1,25 @@
>> +/* Conditional comapre related functions.
>> +   Copyright (C) 2014-2014 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.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with GCC; see the file COPYING3.  If not see
>> +<http://www.gnu.org/licenses/>.  */
>> +
>> +#ifndef GCC_CCMP_H
>> +#define GCC_CCMP_H
>> +
>> +extern bool ccmp_insn_p (rtx);
>> +
>> +#endif  /* GCC_CCMP_H  */
>> diff --git a/gcc/recog.c b/gcc/recog.c
>> index 8d10a4f..b53a28c 100644
>> --- a/gcc/recog.c
>> +++ b/gcc/recog.c
>> @@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "tree-pass.h"
>>  #include "df.h"
>>  #include "insn-codes.h"
>> +#include "ccmp.h"
>>
>>  #ifndef STACK_PUSH_CODE
>>  #ifdef STACK_GROWS_DOWNWARD
>> @@ -577,7 +578,8 @@ simplify_while_replacing (rtx *loc, rtx to, rtx object,
>>    enum rtx_code code = GET_CODE (x);
>>    rtx new_rtx = NULL_RTX;
>>
>> -  if (SWAPPABLE_OPERANDS_P (x)
>> +  /* Do not swap compares in conditional compare instruction.  */
>> +  if (SWAPPABLE_OPERANDS_P (x) && !ccmp_insn_p (object)
>>        && swap_commutative_operands_p (XEXP (x, 0), XEXP (x, 1)))
>>      {
>>        validate_unshare_change (object, loc,
>>
>
>

Reply via email to