Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-19 Thread Richard Biener 
On Thu, Nov 13, 2014 at 7:57 PM, Ulrich Weigand uweig...@de.ibm.com wrote:
 Richard Henderson wrote:
 On 11/12/2014 09:41 PM, Ulrich Weigand wrote:
  * optabs.c (prepare_operand): Gracefully fail if the mode of X
  does not match the operand mode expected by the insn pattern.

 This is ok.

 I've checked this in now, thanks.

 I wondered whether s390 benefit from following i386 in allowing
 movcc to accept immediate operands pre-reload.  This could then
 be re-used by cstorecc4 to allow any CC mode to expand to LOCR
 when available.

 Well, we already compile

 int test (long x, long y)
 {
   return x  y;
 }

 to

 cgr %r2,%r3 # 44*cmpdi_ccs/1[length = 4]
 lhi %r1,0   # 43*movsi_zarch/1  [length = 4]
 lhi %r2,1   # 7 *movsi_zarch/1  [length = 4]
 locrnl  %r2,%r1 # 45*movsicc/2  [length = 4]
 lgfr%r2,%r2 # 17*extendsidi2/1  [length = 4]
 br  %r14# 50return  [length = 2]

 with -march=z196 or higher.  The LOCR is not introduced via
 cstorecc4, but instead directly by noce_emit_cmove via
 emit_conditional_move.  It doesn't seem to matter that the
 patterns don't accept immediate operands; those are loaded
 into registers by maybe_legitimize_operand, which already
 tries a copy_to_mode_reg.

This probably caused bootstrap on s390x-linux to fail as in PR63952
(last checked with rev. 217714).

Richard.

 Bye,
 Ulrich

 --
   Dr. Ulrich Weigand
   GNU/Linux compilers and toolchain
   ulrich.weig...@de.ibm.com

Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-17 Thread H.J. Lu 
On Wed, Nov 12, 2014 at 11:49 PM, Zhenqiang Chen zhenqiang.c...@arm.com wrote:

 -Original Message-
 From: Richard Henderson [mailto:r...@redhat.com]
 Sent: Thursday, November 06, 2014 4:23 PM
 To: Zhenqiang Chen; 'Jan-Benedict Glaw'; Hartmut Penner; Ulrich Weigand;
 Andreas Krebbel
 Cc: gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390
 build)

 On 11/06/2014 08:44 AM, Zhenqiang Chen wrote:
  Hi,
 
  The patch add runtime check to fix s390 build fail
  (https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00050.html).
 
  And there is additional code to workaround s390 cstorecc4 issue.
 
  Bootstrap and no make check regression on X86-64.
  Build s390-linux-gnu and s390x-linux-gnu.
 
  I do not have env to run full s390 tests. Would anyone in the TO list
  help to run the s390 tests?
 
  Thanks!
  -Zhenqiang
 
  ChangeLog:
  2014-11-06  Zhenqiang Chen  zhenqiang.c...@arm.com
 
  * ifcvt.c (noce_emit_cmove, noce_get_alt_condition,
  noce_get_condition):
  Allow CC mode if HAVE_cbranchcc4.
  (noce_emit_store_flag): Change CC REG as cond_complex.
 
  diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index f4002f9..7f7b7c1 100644
  --- a/gcc/ifcvt.c
  +++ b/gcc/ifcvt.c
  @@ -849,7 +849,10 @@ noce_emit_store_flag (struct noce_if_info
  *if_info, rtx x, int reversep,
 enum rtx_code code;
 
 cond_complex = (! general_operand (XEXP (cond, 0), VOIDmode)
  - || ! general_operand (XEXP (cond, 1), VOIDmode));
  + || ! general_operand (XEXP (cond, 1), VOIDmode)
  +/* For s390, CC REG is general_operand.  But cstorecc4
  only
  +   handles CCZ1, which can not handle others like CCU.
  */
  + || GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) ==
 MODE_CC);
 

 I'd like to know more about this.  This seems like a mistake in the
 backend.

  +#ifdef HAVE_cbranchcc4
  +  if (GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
  + || cmp_b != const0_rtx
  + || !(HAVE_cbranchcc4))
  +#endif

 Please add

 #ifndef HAVE_cbranchcc4
 # define HAVE_cbranchcc4
 #endif

 at the top of ifcvt.c along with all of the others.  Then use normal C
 tests
 instead of preprocessor tests.

 Thanks for the comment.

  +  int allow_cc_mode = false;
  +#ifdef HAVE_cbranchcc4
  +  allow_cc_mode = HAVE_cbranchcc4;
  +#endif

 E.g. this becomes

   bool allow_cc_mode = HAVE_cbranchcc4;

 After adding HAVE_cbranchcc4, we can just use HAVE_cbranchcc4. No need to
 add a local variable allow_cc_mode.

 Here is the updated patch.

 diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
 index f4002f9..21f08c2 100644
 --- a/gcc/ifcvt.c
 +++ b/gcc/ifcvt.c
 @@ -75,6 +75,10 @@
 + 1)
  #endif

 +#ifndef HAVE_cbranchcc4
 +#define HAVE_cbranchcc4 0
 +#endif
 +
  #define IFCVT_MULTIPLE_DUMPS 1

  #define NULL_BLOCK ((basic_block) NULL)
 @@ -1459,10 +1463,16 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx
 x, enum rtx_code code,
end_sequence ();
  }

 -  /* Don't even try if the comparison operands are weird.  */
 +  /* Don't even try if the comparison operands are weird
 + except that the target supports cbranchcc4.  */
if (! general_operand (cmp_a, GET_MODE (cmp_a))
|| ! general_operand (cmp_b, GET_MODE (cmp_b)))
 -return NULL_RTX;
 +{
 +  if (!(HAVE_cbranchcc4)
 + || GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
 + || cmp_b != const0_rtx)
 +   return NULL_RTX;
 +}

  #if HAVE_conditional_move
unsignedp = (code == LTU || code == GEU
 @@ -1909,7 +1919,7 @@ noce_get_alt_condition (struct noce_if_info *if_info,
 rtx target,
  }

cond = canonicalize_condition (if_info-jump, cond, reverse,
 -earliest, target, false, true);
 +earliest, target, HAVE_cbranchcc4, true);
if (! cond || ! reg_mentioned_p (target, cond))
  return NULL;

 @@ -2401,7 +2411,7 @@ noce_get_condition (rtx_insn *jump, rtx_insn
 **earliest, bool then_else_reversed
/* Otherwise, fall back on canonicalize_condition to do the dirty
   work of manipulating MODE_CC values and COMPARE rtx codes.  */
tmp = canonicalize_condition (jump, cond, reverse, earliest,
 -   NULL_RTX, false, true);
 +   NULL_RTX, HAVE_cbranchcc4, true);

/* We don't handle side-effects in the condition, like handling
   REG_INC notes and making sure no duplicate conditions are emitted.  */




This caused:

FAIL: gcc.dg/torture/fp-int-convert-float128.c   -O0  execution test
FAIL: gcc.dg/torture/fp-int-convert-float128.c   -O1  execution test
FAIL: gcc.dg/torture/fp-int-convert-float128.c   -O2  execution test
FAIL: gcc.dg/torture/fp-int-convert-float128.c   -O2 -flto
-fno-use-linker-plugin -flto-partition=none  execution test
FAIL: gcc.dg/torture/fp-int-convert-float128.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects  execution test
FAIL: gcc.dg/torture/fp-int-convert-float128.c

Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-13 Thread Ulrich Weigand 
Richard Henderson wrote:
 On 11/12/2014 09:41 PM, Ulrich Weigand wrote:
  * optabs.c (prepare_operand): Gracefully fail if the mode of X
  does not match the operand mode expected by the insn pattern.
 
 This is ok.

I've checked this in now, thanks.

 I wondered whether s390 benefit from following i386 in allowing
 movcc to accept immediate operands pre-reload.  This could then
 be re-used by cstorecc4 to allow any CC mode to expand to LOCR
 when available.

Well, we already compile

int test (long x, long y)
{
  return x  y;
}

to

cgr %r2,%r3 # 44*cmpdi_ccs/1[length = 4]
lhi %r1,0   # 43*movsi_zarch/1  [length = 4]
lhi %r2,1   # 7 *movsi_zarch/1  [length = 4]
locrnl  %r2,%r1 # 45*movsicc/2  [length = 4]
lgfr%r2,%r2 # 17*extendsidi2/1  [length = 4]
br  %r14# 50return  [length = 2]

with -march=z196 or higher.  The LOCR is not introduced via
cstorecc4, but instead directly by noce_emit_cmove via
emit_conditional_move.  It doesn't seem to matter that the
patterns don't accept immediate operands; those are loaded
into registers by maybe_legitimize_operand, which already
tries a copy_to_mode_reg.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com

Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-12 Thread Ulrich Weigand 
Zhenqiang Chen wrote:

 Function noce_emit_store_flag tries to generate instruction to store flag by
 emit_store_flag for general_operand. For s390, CCU is a general _operand,
 but can not match cstorecc4, then it tries to generate a register move
 instruction from CCU to CCZ1, which will trigger an ICE.

OK, I think I see the problem now.  emit_store_flag calls emit_cstore, which
calls prepare_operand, which does:

  if (!insn_operand_matches (icode, opnum, x))
{
  if (reload_completed)
return NULL_RTX;
  x = copy_to_mode_reg (insn_data[(int) icode].operand[opnum].mode, x);
}

The insn_operand_matches call will indeed fail since x has CCUmode, but
the operand only accepts CCZ1mode for this insn.

However, it is a bug to use copy_to_mode_reg with a mode different
from the mode of the RTX to be loaded (unless that RTX is a constant
of VOIDmode); copy_to_mode_reg will simply abort in that case.

Usually, this doesn't happen since prepare_operand is called on insns that
were already selected according to the operand mode.  However, this is
weakened for MODE_CC modes in emit_store_flag:

 machine_mode optab_mode = mclass == MODE_CC ? CCmode : compare_mode;
 icode = optab_handler (cstore_optab, optab_mode);

So the assumption seems to be that a cstorecc4 insn must accept *any*
MODE_CC mode, or else prepare_operand will abort.  I don't think this
requirement is really useful; on s390 there are good reasons to only
accept certain MODE_CC modes and ask the middle-end to fall back to
the generic implemention for others.

Thus I'd propose to add a mode check to prepare_operand and simply fail
instead of aborting if the mode doesn't match what the insn expects.
The appended patch does this; it fixes the ICEs when adding your patch.

Tested on s390x-ibm-linux.

Richard, does this look reasonable to you?

Bye,
Ulrich

ChangeLog:

* optabs.c (prepare_operand): Gracefully fail if the mode of X
does not match the operand mode expected by the insn pattern.

Index: gcc/optabs.c
===
*** gcc/optabs.c(revision 217416)
--- gcc/optabs.c(working copy)
*** prepare_operand (enum insn_code icode, r
*** 4308,4316 
  
if (!insn_operand_matches (icode, opnum, x))
  {
if (reload_completed)
return NULL_RTX;
!   x = copy_to_mode_reg (insn_data[(int) icode].operand[opnum].mode, x);
  }
  
return x;
--- 4308,4319 
  
if (!insn_operand_matches (icode, opnum, x))
  {
+   machine_mode op_mode = insn_data[(int) icode].operand[opnum].mode;
if (reload_completed)
return NULL_RTX;
!   if (GET_MODE (x) != op_mode  GET_MODE (x) != VOIDmode)
!   return NULL_RTX;
!   x = copy_to_mode_reg (op_mode, x);
  }
  
return x;



-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com

RE: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-12 Thread Zhenqiang Chen 

 -Original Message-
 From: Richard Henderson [mailto:r...@redhat.com]
 Sent: Thursday, November 06, 2014 4:23 PM
 To: Zhenqiang Chen; 'Jan-Benedict Glaw'; Hartmut Penner; Ulrich Weigand;
 Andreas Krebbel
 Cc: gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390
build)
 
 On 11/06/2014 08:44 AM, Zhenqiang Chen wrote:
  Hi,
 
  The patch add runtime check to fix s390 build fail
  (https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00050.html).
 
  And there is additional code to workaround s390 cstorecc4 issue.
 
  Bootstrap and no make check regression on X86-64.
  Build s390-linux-gnu and s390x-linux-gnu.
 
  I do not have env to run full s390 tests. Would anyone in the TO list
  help to run the s390 tests?
 
  Thanks!
  -Zhenqiang
 
  ChangeLog:
  2014-11-06  Zhenqiang Chen  zhenqiang.c...@arm.com
 
  * ifcvt.c (noce_emit_cmove, noce_get_alt_condition,
  noce_get_condition):
  Allow CC mode if HAVE_cbranchcc4.
  (noce_emit_store_flag): Change CC REG as cond_complex.
 
  diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index f4002f9..7f7b7c1 100644
  --- a/gcc/ifcvt.c
  +++ b/gcc/ifcvt.c
  @@ -849,7 +849,10 @@ noce_emit_store_flag (struct noce_if_info
  *if_info, rtx x, int reversep,
 enum rtx_code code;
 
 cond_complex = (! general_operand (XEXP (cond, 0), VOIDmode)
  - || ! general_operand (XEXP (cond, 1), VOIDmode));
  + || ! general_operand (XEXP (cond, 1), VOIDmode)
  +/* For s390, CC REG is general_operand.  But cstorecc4
  only
  +   handles CCZ1, which can not handle others like CCU.
  */
  + || GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) ==
 MODE_CC);
 
 
 I'd like to know more about this.  This seems like a mistake in the
backend.
 
  +#ifdef HAVE_cbranchcc4
  +  if (GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
  + || cmp_b != const0_rtx
  + || !(HAVE_cbranchcc4))
  +#endif
 
 Please add
 
 #ifndef HAVE_cbranchcc4
 # define HAVE_cbranchcc4
 #endif
 
 at the top of ifcvt.c along with all of the others.  Then use normal C
tests
 instead of preprocessor tests.

Thanks for the comment.
 
  +  int allow_cc_mode = false;
  +#ifdef HAVE_cbranchcc4
  +  allow_cc_mode = HAVE_cbranchcc4;
  +#endif
 
 E.g. this becomes
 
   bool allow_cc_mode = HAVE_cbranchcc4;

After adding HAVE_cbranchcc4, we can just use HAVE_cbranchcc4. No need to
add a local variable allow_cc_mode.

Here is the updated patch.

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index f4002f9..21f08c2 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -75,6 +75,10 @@
+ 1)
 #endif
 
+#ifndef HAVE_cbranchcc4
+#define HAVE_cbranchcc4 0
+#endif
+
 #define IFCVT_MULTIPLE_DUMPS 1
 
 #define NULL_BLOCK ((basic_block) NULL)
@@ -1459,10 +1463,16 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx
x, enum rtx_code code,
   end_sequence ();
 }
 
-  /* Don't even try if the comparison operands are weird.  */
+  /* Don't even try if the comparison operands are weird
+ except that the target supports cbranchcc4.  */
   if (! general_operand (cmp_a, GET_MODE (cmp_a))
   || ! general_operand (cmp_b, GET_MODE (cmp_b)))
-return NULL_RTX;
+{
+  if (!(HAVE_cbranchcc4)
+ || GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
+ || cmp_b != const0_rtx)
+   return NULL_RTX;
+}
 
 #if HAVE_conditional_move
   unsignedp = (code == LTU || code == GEU
@@ -1909,7 +1919,7 @@ noce_get_alt_condition (struct noce_if_info *if_info,
rtx target,
 }
 
   cond = canonicalize_condition (if_info-jump, cond, reverse,
-earliest, target, false, true);
+earliest, target, HAVE_cbranchcc4, true);
   if (! cond || ! reg_mentioned_p (target, cond))
 return NULL;
 
@@ -2401,7 +2411,7 @@ noce_get_condition (rtx_insn *jump, rtx_insn
**earliest, bool then_else_reversed
   /* Otherwise, fall back on canonicalize_condition to do the dirty
  work of manipulating MODE_CC values and COMPARE rtx codes.  */
   tmp = canonicalize_condition (jump, cond, reverse, earliest,
-   NULL_RTX, false, true);
+   NULL_RTX, HAVE_cbranchcc4, true);
 
   /* We don't handle side-effects in the condition, like handling
  REG_INC notes and making sure no duplicate conditions are emitted.  */

Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-12 Thread Richard Henderson 
On 11/13/2014 08:49 AM, Zhenqiang Chen wrote:
 After adding HAVE_cbranchcc4, we can just use HAVE_cbranchcc4. No need to
 add a local variable allow_cc_mode.
 
 Here is the updated patch.

This is ok.

Since I've already approved Ulrich's s390 fix, there should not be a
problem there for long.


r~

RE: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-11 Thread Zhenqiang Chen 


 -Original Message-
 From: Ulrich Weigand [mailto:uweig...@de.ibm.com]
 Sent: Friday, November 07, 2014 12:11 AM
 To: Richard Henderson
 Cc: Zhenqiang Chen; 'Jan-Benedict Glaw'; Hartmut Penner; Andreas Krebbel;
 gcc-patches@gcc.gnu.org
 Subject: Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390
build)
 
 Richard Henderson wrote:
  On 11/06/2014 08:44 AM, Zhenqiang Chen wrote:
   Hi,
  
   The patch add runtime check to fix s390 build fail
   (https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00050.html).
  
   And there is additional code to workaround s390 cstorecc4 issue.
  
   Bootstrap and no make check regression on X86-64.
   Build s390-linux-gnu and s390x-linux-gnu.
  
   I do not have env to run full s390 tests. Would anyone in the TO
   list help to run the s390 tests?
  
   Thanks!
   -Zhenqiang
  
   ChangeLog:
   2014-11-06  Zhenqiang Chen  zhenqiang.c...@arm.com
  
 * ifcvt.c (noce_emit_cmove, noce_get_alt_condition,
   noce_get_condition):
 Allow CC mode if HAVE_cbranchcc4.
 (noce_emit_store_flag): Change CC REG as cond_complex.
  
   diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index f4002f9..7f7b7c1 100644
   --- a/gcc/ifcvt.c
   +++ b/gcc/ifcvt.c
   @@ -849,7 +849,10 @@ noce_emit_store_flag (struct noce_if_info
   *if_info, rtx x, int reversep,
  enum rtx_code code;
  
  cond_complex = (! general_operand (XEXP (cond, 0), VOIDmode)
   -   || ! general_operand (XEXP (cond, 1), VOIDmode));
   +   || ! general_operand (XEXP (cond, 1), VOIDmode)
   +  /* For s390, CC REG is general_operand.  But cstorecc4
   only
   + handles CCZ1, which can not handle others like CCU.
   */
   +   || GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) ==
 MODE_CC);
  
 
  I'd like to know more about this.  This seems like a mistake in the
backend.
 
 We do indeed allow the CC register as general_operand, since it has a
 register class of CC_REGS.  This has been in place for over 10 years now
and
 was a deliberate decision to allow for pseudos that carry a condition
value to
 be allocated to the actual CC register or some GPR, depending on reload's
 choices.  As long as moves and (secondary) reloads are fully supported
(and
 they should be), this should be OK ...
 
 Zhenqiang, what specific problem are you seeing?

Function noce_emit_store_flag tries to generate instruction to store flag by
emit_store_flag for general_operand. For s390, CCU is a general _operand,
but can not match cstorecc4, then it tries to generate a register move
instruction from CCU to CCZ1, which will trigger an ICE.

With the patch (removing the workaround for s390), I got lots of ICE when
building libgcc.

Thank!
-Zhenqiang

 Bye,
 Ulrich
 
 --
   Dr. Ulrich Weigand
   GNU/Linux compilers and toolchain
   ulrich.weig...@de.ibm.com

Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-07 Thread Ulrich Weigand 
Richard Henderson wrote:
 On 11/06/2014 05:10 PM, Ulrich Weigand wrote:
  +  /* For s390, CC REG is general_operand.  But cstorecc4
  only
  + handles CCZ1, which can not handle others like CCU.
  */
  +   || GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == MODE_CC);
   
 
  I'd like to know more about this.  This seems like a mistake in the 
  backend.
  
  We do indeed allow the CC register as general_operand, since it has a
  register class of CC_REGS.
 
 I rather meant that cstorecc4 only handles some MODE_CC, not that
 CC is a general_operand, that seems questionable.

Well, it's only for CCZ1mode that we can implement cstorecc4 using a simple
IPM / shift sequence.  In order to handle generic MODE_CC modes, we'd have
to implement a mask check, something like IPM / shift / load immediate
/ shift / and-immediate.  This will usually be slower than then default
branch sequence generated by the middle end.

[ On z196 and more recent machines, we already use LOCR to select one of
two immediates (in registers), generated via the movmodecc expander;
this can handle generic MODE_CC modes. ]

But no matter the rationale, I still don't see what the original problem is;
the pattern describes the mode it can support -- shouldn't this be enough
for the middle end to know whether it is available, without requiring any
further special-case checks?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com

Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-06 Thread Richard Henderson 
On 11/06/2014 08:44 AM, Zhenqiang Chen wrote:
 Hi,
 
 The patch add runtime check to fix s390 build fail
 (https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00050.html).
 
 And there is additional code to workaround s390 cstorecc4 issue.
 
 Bootstrap and no make check regression on X86-64.
 Build s390-linux-gnu and s390x-linux-gnu.
 
 I do not have env to run full s390 tests. Would anyone in the TO list help
 to run the s390 tests?
 
 Thanks!
 -Zhenqiang
 
 ChangeLog:
 2014-11-06  Zhenqiang Chen  zhenqiang.c...@arm.com
 
   * ifcvt.c (noce_emit_cmove, noce_get_alt_condition,
 noce_get_condition):
   Allow CC mode if HAVE_cbranchcc4.
   (noce_emit_store_flag): Change CC REG as cond_complex.
 
 diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
 index f4002f9..7f7b7c1 100644
 --- a/gcc/ifcvt.c
 +++ b/gcc/ifcvt.c
 @@ -849,7 +849,10 @@ noce_emit_store_flag (struct noce_if_info *if_info, rtx
 x, int reversep,
enum rtx_code code;
  
cond_complex = (! general_operand (XEXP (cond, 0), VOIDmode)
 -   || ! general_operand (XEXP (cond, 1), VOIDmode));
 +   || ! general_operand (XEXP (cond, 1), VOIDmode)
 +  /* For s390, CC REG is general_operand.  But cstorecc4
 only
 + handles CCZ1, which can not handle others like CCU.
 */
 +   || GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == MODE_CC);
  

I'd like to know more about this.  This seems like a mistake in the backend.

 +#ifdef HAVE_cbranchcc4
 +  if (GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
 +   || cmp_b != const0_rtx
 +   || !(HAVE_cbranchcc4))
 +#endif

Please add

#ifndef HAVE_cbranchcc4
# define HAVE_cbranchcc4
#endif

at the top of ifcvt.c along with all of the others.  Then use normal C tests
instead of preprocessor tests.

 +  int allow_cc_mode = false;
 +#ifdef HAVE_cbranchcc4
 +  allow_cc_mode = HAVE_cbranchcc4;
 +#endif

E.g. this becomes

  bool allow_cc_mode = HAVE_cbranchcc4;


r~

Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-06 Thread Ulrich Weigand 
Richard Henderson wrote:
 On 11/06/2014 08:44 AM, Zhenqiang Chen wrote:
  Hi,
  
  The patch add runtime check to fix s390 build fail
  (https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00050.html).
  
  And there is additional code to workaround s390 cstorecc4 issue.
  
  Bootstrap and no make check regression on X86-64.
  Build s390-linux-gnu and s390x-linux-gnu.
  
  I do not have env to run full s390 tests. Would anyone in the TO list help
  to run the s390 tests?
  
  Thanks!
  -Zhenqiang
  
  ChangeLog:
  2014-11-06  Zhenqiang Chen  zhenqiang.c...@arm.com
  
  * ifcvt.c (noce_emit_cmove, noce_get_alt_condition,
  noce_get_condition):
  Allow CC mode if HAVE_cbranchcc4.
  (noce_emit_store_flag): Change CC REG as cond_complex.
  
  diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
  index f4002f9..7f7b7c1 100644
  --- a/gcc/ifcvt.c
  +++ b/gcc/ifcvt.c
  @@ -849,7 +849,10 @@ noce_emit_store_flag (struct noce_if_info *if_info, rtx
  x, int reversep,
 enum rtx_code code;
   
 cond_complex = (! general_operand (XEXP (cond, 0), VOIDmode)
  - || ! general_operand (XEXP (cond, 1), VOIDmode));
  + || ! general_operand (XEXP (cond, 1), VOIDmode)
  +/* For s390, CC REG is general_operand.  But cstorecc4
  only
  +   handles CCZ1, which can not handle others like CCU.
  */
  + || GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == MODE_CC);
   
 
 I'd like to know more about this.  This seems like a mistake in the backend.

We do indeed allow the CC register as general_operand, since it has a
register class of CC_REGS.  This has been in place for over 10 years now
and was a deliberate decision to allow for pseudos that carry a condition
value to be allocated to the actual CC register or some GPR, depending on
reload's choices.  As long as moves and (secondary) reloads are fully
supported (and they should be), this should be OK ...

Zhenqiang, what specific problem are you seeing?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com

Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-06 Thread Richard Henderson 
On 11/06/2014 05:10 PM, Ulrich Weigand wrote:
 +/* For s390, CC REG is general_operand.  But cstorecc4
 only
 +   handles CCZ1, which can not handle others like CCU.
 */
 + || GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == MODE_CC);
  

 I'd like to know more about this.  This seems like a mistake in the backend.
 
 We do indeed allow the CC register as general_operand, since it has a
 register class of CC_REGS.

I rather meant that cstorecc4 only handles some MODE_CC, not that
CC is a general_operand, that seems questionable.


r~

[PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

2014-11-05 Thread Zhenqiang Chen 
Hi,

The patch add runtime check to fix s390 build fail
(https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00050.html).

And there is additional code to workaround s390 cstorecc4 issue.

Bootstrap and no make check regression on X86-64.
Build s390-linux-gnu and s390x-linux-gnu.

I do not have env to run full s390 tests. Would anyone in the TO list help
to run the s390 tests?

Thanks!
-Zhenqiang

ChangeLog:
2014-11-06  Zhenqiang Chen  zhenqiang.c...@arm.com

* ifcvt.c (noce_emit_cmove, noce_get_alt_condition,
noce_get_condition):
Allow CC mode if HAVE_cbranchcc4.
(noce_emit_store_flag): Change CC REG as cond_complex.

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index f4002f9..7f7b7c1 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -849,7 +849,10 @@ noce_emit_store_flag (struct noce_if_info *if_info, rtx
x, int reversep,
   enum rtx_code code;
 
   cond_complex = (! general_operand (XEXP (cond, 0), VOIDmode)
- || ! general_operand (XEXP (cond, 1), VOIDmode));
+ || ! general_operand (XEXP (cond, 1), VOIDmode)
+/* For s390, CC REG is general_operand.  But cstorecc4
only
+   handles CCZ1, which can not handle others like CCU.
*/
+ || GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == MODE_CC);
 
   /* If earliest == jump, or when the condition is complex, try to
  build the store_flag insn directly.  */
@@ -1459,10 +1462,18 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx
x, enum rtx_code code,
   end_sequence ();
 }
 
-  /* Don't even try if the comparison operands are weird.  */
+  /* Don't even try if the comparison operands are weird
+ except that the target supports cbranchcc4.  */
   if (! general_operand (cmp_a, GET_MODE (cmp_a))
   || ! general_operand (cmp_b, GET_MODE (cmp_b)))
-return NULL_RTX;
+{
+#ifdef HAVE_cbranchcc4
+  if (GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
+ || cmp_b != const0_rtx
+ || !(HAVE_cbranchcc4))
+#endif
+   return NULL_RTX;
+}
 
 #if HAVE_conditional_move
   unsignedp = (code == LTU || code == GEU
@@ -1788,6 +1799,11 @@ noce_get_alt_condition (struct noce_if_info *if_info,
rtx target,
   rtx cond, set;
   rtx_insn *insn;
   int reverse;
+  int allow_cc_mode = false;
+#ifdef HAVE_cbranchcc4
+  allow_cc_mode = HAVE_cbranchcc4;
+#endif
+
 
   /* If target is already mentioned in the known condition, return it.  */
   if (reg_mentioned_p (target, if_info-cond))
@@ -1909,7 +1925,7 @@ noce_get_alt_condition (struct noce_if_info *if_info,
rtx target,
 }
 
   cond = canonicalize_condition (if_info-jump, cond, reverse,
-earliest, target, false, true);
+earliest, target, allow_cc_mode, true);
   if (! cond || ! reg_mentioned_p (target, cond))
 return NULL;
 
@@ -2365,6 +2381,10 @@ noce_get_condition (rtx_insn *jump, rtx_insn
**earliest, bool then_else_reversed
 {
   rtx cond, set, tmp;
   bool reverse;
+  int allow_cc_mode = false;
+#ifdef HAVE_cbranchcc4
+  allow_cc_mode = HAVE_cbranchcc4;
+#endif
 
   if (! any_condjump_p (jump))
 return NULL_RTX;
@@ -2401,7 +2421,7 @@ noce_get_condition (rtx_insn *jump, rtx_insn
**earliest, bool then_else_reversed
   /* Otherwise, fall back on canonicalize_condition to do the dirty
  work of manipulating MODE_CC values and COMPARE rtx codes.  */
   tmp = canonicalize_condition (jump, cond, reverse, earliest,
-   NULL_RTX, false, true);
+   NULL_RTX, allow_cc_mode, true);
 
   /* We don't handle side-effects in the condition, like handling
  REG_INC notes and making sure no duplicate conditions are emitted.  */