This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, cilk, C++] Fix cilk testsuite failure


On 01/15/14 21:23, Andrew Pinski wrote:
On Wed, Jan 15, 2014 at 10:50 AM, Steve Ellcey <sellcey@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.
Or maybe during expansion.

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]