Hi,

I am trying to understand why the MIPS backend's handling of
comparison modes seems to be different from what rtl.texi says:

  The mode of the comparison operation is independent of the mode
  of the data being compared.  If the comparison operation is being tested
  (e.g., the first operand of an @code{if_then_else}), the mode must be
  @code{VOIDmode}.

In the MIPS backend conditional branches based on integer comparisons
are valid only if the mode of the comparison matches the mode of the
operands, e.g. in 64-bit mode (if_then_else (eq:DI (reg:DI 1)
(const_int 0)) ...).

This is relevant because combine could optimize this:

  (set (reg:DI 2) (and:DI (reg:DI 1) (const_int 1)))
  (set (reg:SI 3) (truncate:SI (reg:DI 2)))
  (set (pc) (if_then_else (eq:SI (reg:SI 3) (const_int 0)) ...))

into this:

  (set (reg:DI 2) (and:DI (regDI 1) (const_int 1)))
  (set (pc) (if_then_else (eq:SI (reg:DI 2) (const_int 0)) ...))

I think this is a valid transformation on MIPS64 too.  In fact I
believe that as long as we compare valid SImode or DImode values the
64-bit comparison should work fine.

If I remove the comparison modes from the conditional branch patterns
regression testing on mips64-elf does not show any problem (diff is
attached for reference).  When I however compare the assembly output
it is clear that some of the changes are incorrect.  This is
a simplified testcase illustrating the problem:

  f (long long a)
  {
    if ((a & 0xffffffff) != 0)
      abort ();
  }
  
  long long a = 0x1234567800000000LL;
  main ()
  {
    f (a);
  }

What happens is that combine.c:simplify_comparison <AND> noticing the
zero-extension transforms this:

  (ne:DI (and:DI (reg:DI 4) (const_int 0xffffffff)) (const_int 0))

into this:

  (ne:DI (reg:SI 4) (const_int 0))

And now we do allow comparison like this in conditional branches.

On targets where DI->SI mode-switch is not a nop I think the
transformation in simplify_comparison is incorrect.  We need an
explicit truncate here to enforce valid SI values:

  (ne:DI (truncate:SI (reg:DI 4)) (const_int 0)).

Now if I am correct and this last thing is really a bug then the
obvious question is whether it has anything to do with the more
restrictive form for conditional branches on MIPS64?  And of course if
I fix it then whether it would be OK to lift the mode restrictions in
the conditional branch patterns.

Thanks,
Adam

Index: mips.md
===================================================================
--- mips.md     (revision 107758)
+++ mips.md     (working copy)
@@ -4291,7 +4291,7 @@
 (define_insn "*branch_zero<mode>"
   [(set (pc)
        (if_then_else
-        (match_operator:GPR 0 "comparison_operator"
+        (match_operator 0 "comparison_operator"
                             [(match_operand:GPR 2 "register_operand" "d")
                              (const_int 0)])
         (label_ref (match_operand 1 "" ""))
@@ -4311,7 +4311,7 @@
 (define_insn "*branch_zero<mode>_inverted"
   [(set (pc)
        (if_then_else
-        (match_operator:GPR 0 "comparison_operator"
+        (match_operator 0 "comparison_operator"
                             [(match_operand:GPR 2 "register_operand" "d")
                              (const_int 0)])
         (pc)
@@ -4333,7 +4333,7 @@
 (define_insn "*branch_equality<mode>"
   [(set (pc)
        (if_then_else
-        (match_operator:GPR 0 "equality_operator"
+        (match_operator 0 "equality_operator"
                             [(match_operand:GPR 2 "register_operand" "d")
                              (match_operand:GPR 3 "register_operand" "d")])
         (label_ref (match_operand 1 "" ""))
@@ -4353,7 +4353,7 @@
 (define_insn "*branch_equality<mode>_inverted"
   [(set (pc)
        (if_then_else
-        (match_operator:GPR 0 "equality_operator"
+        (match_operator 0 "equality_operator"
                             [(match_operand:GPR 2 "register_operand" "d")
                              (match_operand:GPR 3 "register_operand" "d")])
         (pc)

Reply via email to