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, >> > >
