On Wed, Jan 15, 2014 at 10:50 AM, Steve Ellcey <sell...@mips.com> wrote: > The test c-c++-common/cilk-plus/AN/builtin_func_double2.c is failing on MIPS > when compiled with C++ but not when compiled with C because GCC is generating > an illegal assembly language instruction during the C++ compilation. The > reason this works in C but fails for C++ is because the > __sec_reduce_any_nonzero call is creating a conditional assignment expression > and in C the condition used in that expression is type integer but in C++ > it is type boolean. > > In c/c-array-notation.c we have: > > case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_ZERO: > case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO: > case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO: > case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO: > new_var_type = integer_type_node; > break; > > new_var_type is used as the type of a variable used as the condition 'C' in > "foo = C ? 1 : 0". > > But in C++ we use a boolean type instead of an integer type for the > condition. Thus, in cp/cp-array-notation.c we have: > > case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_ZERO: > case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO: > case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO: > case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO: > new_var_type = boolean_type_node; > break; > > The reason this matters is that on MIPS a floating point conditional > register ($fcc0) can hold a boolean type but not an integer type > but this register cannot be used in a conditional move. So when the > C++ code is generated GCC will create an instruction that looks like: > > movz $4,$6,$fcc0 > > And this is illegal. I would like to fix this by using integer_type_node > for C++ like C is doing and I have attached a tested patch that does > this. It fixes the bug and caused no regressions on mips-mti-linux-gnu. > > One could argue that a better fix would be checking the legality of the > conditional move in expr.c and not generating an illegal one, however the > current GCC conditional move code checks for the type (mode) of the value > being moved but has no check on the type or mode of the conditional being > used to control the move so doing this would probably mean adding a new > target predicate. Since no other tests are failing with this problem I > would rather tweak the cilk code instead. > > Tested on mips-mti-linux-gnu with no regressions. > > OK to checkin?
No, this really should be fixed in the target side. In fact for MIPS there is an instruction which does a conditional move based on the FP CC register (movt/movf). So you need to look into why the wrong pattern is being selected instead. Thanks, Andrew Pinski > > Steve Ellcey > sell...@mips.com > > > 2014-01-15 Steve Ellcey <sell...@mips.com> > > PR target/59462 > * cp-array-notation.c (expand_sec_reduce_builtin): Change conditional > type. > > > diff --git a/gcc/cp/cp-array-notation.c b/gcc/cp/cp-array-notation.c > index 65b8bcb..6cba0b4 100644 > --- a/gcc/cp/cp-array-notation.c > +++ b/gcc/cp/cp-array-notation.c > @@ -274,7 +274,7 @@ expand_sec_reduce_builtin (tree an_builtin_fn, tree > *new_var) > case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO: > case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO: > case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO: > - new_var_type = boolean_type_node; > + new_var_type = integer_type_node; > break; > case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND: > case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND: >
