The patch is discarded since I can not reproduce the issue with the latest trunk.
Thanks! -Zhenqiang > -----Original Message----- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Zhenqiang Chen > Sent: Thursday, June 26, 2014 3:21 PM > To: Richard Earnshaw > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH, 3/10] skip swapping operands used in ccmp > > 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, > >> > > > >
