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

Andrew Pinski pinskia@gmail.com
Thu Jan 16 21:16:00 GMT 2014


On Thu, Jan 16, 2014 at 12:37 PM, Steve Ellcey <sellcey@mips.com> wrote:
> On Wed, 2014-01-15 at 22:47 -0700, Jeff Law wrote:
>> On 01/15/14 21:23, Andrew Pinski wrote:
>
>> >> 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
>
> Well, I thought it was worth a shot.  Looking at the instruction causing
> the problem we have the RTX:
>
>             (insn:TI 76 79 98 (set (reg:SI 2 $2 [orig:228 D.1939+-3 ] [228])
>                     (if_then_else:SI (ne:SI (reg:CC 67 $fcc0)
>                             (const_int 0 [0]))
>                         (reg:SI 2 $2 [orig:228 D.1939+-3 ] [228])
>                         (reg:SI 4 $4 [246]))) 609 {*movsi_on_cc}
>                  (nil))
>
> And it is being handled by the define_insn:
>
> ;; MIPS4 Conditional move instructions.
>
> (define_insn "*mov<GPR:mode>_on_<MOVECC:mode>"
>   [(set (match_operand:GPR 0 "register_operand" "=d,d")
>         (if_then_else:GPR
>          (match_operator 4 "equality_operator"
>                 [(match_operand:MOVECC 1 "register_operand" "<MOVECC:reg>,<MOVECC:reg>")
>                  (const_int 0)])
>          (match_operand:GPR 2 "reg_or_0_operand" "dJ,0")
>          (match_operand:GPR 3 "reg_or_0_operand" "0,dJ")))]
>   "ISA_HAS_CONDMOVE"
>   "@
>     mov%T4\t%0,%z2,%1
>     mov%t4\t%0,%z3,%1"
>   [(set_attr "type" "condmove")
>    (set_attr "mode" "<GPR:MODE>")])
>
> %T4 is using the mode of operand 4 to determine if it should generate
> movz or movf.  If that mode is CC it uses movf/movt and otherwise it uses
> movz/movn.  I tried tweaking the conditional move define_insn to use
> %T1 and %t1 instead of %T4 and %t4 but that caused regressions because the
> %T code is also looking at the node to see if it is a NE or a EQ.
>
> Given that we have a CC reg type is it reasonable/correct that the ne
> comparision is SI mode and not CC mode?

I think the following patch to mips.c should fix the issue:
@@ -8092,7 +8125,7 @@ mips_print_operand (FILE *file, rtx op, int letter)
     case 't':
       {
        int truth = (code == NE) == (letter == 'T');
-       fputc ("zfnt"[truth * 2 + (GET_MODE (op) == CCmode)], file);
+       fputc ("zfnt"[truth * 2 + (GET_MODE (XEXP (op, 0)) == CCmode)], file);
       }
       break;

----- CUT ----

So this bug was introduced by:
2013-11-18  Andrew Pinski <apinski@cavium.com>
            Steve Ellcey  <sellcey@mips.com>

        PR target/56552
        * config/mips/mips.md (*mov<GPR:mode>_on_<MOVECC:mode>): Remove
        type restriction from equality_operator on conditonal move.
        (*mov<SCALARF:mode>_on_<MOVECC:mode>): Ditto.
        (*mov<GPR:mode>_on_<GPR2:mode>_ne): New.

I forgot when I submitted that I had fixed up the hard float case
later on as I put the fix as part of my octeon3 (the octeon which fp)
patch.

Thanks,
Andrew Pinski

>
> Steve Ellcey
> sellcey@mips.com
>
>



More information about the Gcc-patches mailing list