On Thu, Jan 16, 2014 at 12:37 PM, Steve Ellcey <sell...@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 <apin...@cavium.com>
Steve Ellcey <sell...@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
> sell...@mips.com
>
>
