diff mbox

RFC: [Patch, PR Bug 60818] - ICE in validate_condition_mode on powerpc*-linux-gnu* ]

Message ID 20160216083058.GA31757@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra Feb. 16, 2016, 8:30 a.m. UTC 
On Tue, Feb 16, 2016 at 05:02:30AM +0000, Rohit Arul Raj D wrote:
> ii) combiner pass converts "gtu" to "ne" and converts the mode to "CC" (from CCUNS).
> set (reg/i:SI 3 3)
>     (if_then_else:SI (ne (reg:CC 166)	-----------------> operator and mode changed
>             (const_int 0 [0]))
>         (reg:SI 168)
>         (const_int 0 [0])))
> 
> (gdb) p debug_rtx (other_insn)
> (insn 11 10 16 2 (set (reg:SI 165 [ D.2339+-3 ])
>         (if_then_else:SI (ne (reg:CC 166)
>                 (const_int 0 [0]))
>             (reg:SI 168)
>             (reg:SI 167))) test.c:7 317 {isel_unsigned_si}
>      (expr_list:REG_DEAD (reg:SI 168)
>         (expr_list:REG_DEAD (reg:SI 167)
>             (expr_list:REG_DEAD (reg:CC 166)
>                 (expr_list:REG_EQUAL (gtu:SI (reg:CC 166)
>                         (const_int 0 [0]))
>                     (nil))))))
> 
> iii) once the instruction match fails, it tries to link back to the pattern stored in the REG_EQUAL note to the SRC. But while doing so, it doesn't change the conditional mode based on the operator which leads to the ICE.

I don't think combine is doing anything wrong here.  Combine is quite
reasonable in making the substitutions.

What's wrong is the rs6000 backend asserting that (gtu (reg:CC)) can't
happen, because obviously it does.  Rather than trying to fix combine,
(where the ICE happens on attempting to validate the insn!), I think
the rs6000 backend should change.  Like so.  Not yet bootstrapped,
but I'm about to fire one off.

	PR target/60818
	* config/rs6000/rs6000.c (validate_condition_mode): Return a
	bool rather than asserting modes as expected.  Update all uses
	to assert.
	* config/rs6000/rs6000-protos.h (validate_condition_mode):
	Update prototype.
	* config/rs6000/predicates.md (branch_comparison_operator):
	Use result of validate_condition_mode.
diff mbox

Patch

diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index a74a6a0..f037b0d 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -1089,13 +1089,11 @@ 
 
 ;; Return 1 if OP is a comparison operation that is valid for a branch
 ;; instruction.  We check the opcode against the mode of the CC value.
-;; validate_condition_mode is an assertion.
 (define_predicate "branch_comparison_operator"
    (and (match_operand 0 "comparison_operator")
 	(and (match_test "GET_MODE_CLASS (GET_MODE (XEXP (op, 0))) == MODE_CC")
 	     (match_test "validate_condition_mode (GET_CODE (op),
-						   GET_MODE (XEXP (op, 0))),
-			  1"))))
+						   GET_MODE (XEXP (op, 0)))"))))
 
 ;; Return 1 if OP is a valid comparison operator for "cbranch" instructions.
 ;; If we're assuming that FP operations cannot generate user-visible traps,
diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h
index d9a6b1f..91822e8 100644
--- a/gcc/config/rs6000/rs6000-protos.h
+++ b/gcc/config/rs6000/rs6000-protos.h
@@ -40,7 +40,7 @@  extern int small_data_operand (rtx, machine_mode);
 extern bool mem_operand_gpr (rtx, machine_mode);
 extern bool toc_relative_expr_p (const_rtx, bool);
 extern bool invalid_e500_subreg (rtx, machine_mode);
-extern void validate_condition_mode (enum rtx_code, machine_mode);
+extern bool validate_condition_mode (enum rtx_code, machine_mode);
 extern bool legitimate_constant_pool_address_p (const_rtx, machine_mode,
 						bool);
 extern bool legitimate_indirect_address_p (rtx, int);
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index ed94c53..3fff9ab 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -17162,7 +17162,7 @@  rs6000_output_load_multiple (rtx operands[3])
    match.  The other alternatives either don't make sense or should
    never be generated.  */
 
-void
+bool
 validate_condition_mode (enum rtx_code code, machine_mode mode)
 {
   gcc_assert ((GET_RTX_CLASS (code) == RTX_COMPARE
@@ -17170,28 +17170,36 @@  validate_condition_mode (enum rtx_code code, machine_mode mode)
 	      && GET_MODE_CLASS (mode) == MODE_CC);
 
   /* These don't make sense.  */
-  gcc_assert ((code != GT && code != LT && code != GE && code != LE)
-	      || mode != CCUNSmode);
+  if (mode == CCUNSmode
+      && (code == GT || code == LT || code == GE || code == LE))
+    return false;
 
-  gcc_assert ((code != GTU && code != LTU && code != GEU && code != LEU)
-	      || mode == CCUNSmode);
+  if (mode != CCUNSmode
+      && (code == GTU || code == LTU || code == GEU || code == LEU))
+    return false;
 
-  gcc_assert (mode == CCFPmode
-	      || (code != ORDERED && code != UNORDERED
-		  && code != UNEQ && code != LTGT
-		  && code != UNGT && code != UNLT
-		  && code != UNGE && code != UNLE));
+  if (mode != CCFPmode
+      && (code == ORDERED || code == UNORDERED
+	  || code == UNEQ || code == LTGT
+	  || code == UNGT || code == UNLT
+	  || code == UNGE || code == UNLE))
+    return false;
 
   /* These should never be generated except for
      flag_finite_math_only.  */
-  gcc_assert (mode != CCFPmode
-	      || flag_finite_math_only
-	      || (code != LE && code != GE
-		  && code != UNEQ && code != LTGT
-		  && code != UNGT && code != UNLT));
+  if (mode == CCFPmode
+      && !flag_finite_math_only
+      && (code == LE || code == GE
+	  || code == UNEQ || code == LTGT
+	  || code == UNGT || code == UNLT))
+    return false;
 
   /* These are invalid; the information is not there.  */
-  gcc_assert (mode != CCEQmode || code == EQ || code == NE);
+  if (mode == CCEQmode
+      && code != EQ && code != NE)
+    return false;
+
+  return true;
 }
 
 
@@ -19583,7 +19591,7 @@  ccr_bit (rtx op, int scc_p)
   cc_regnum = REGNO (reg);
   base_bit = 4 * (cc_regnum - CR0_REGNO);
 
-  validate_condition_mode (code, cc_mode);
+  gcc_assert (validate_condition_mode (code, cc_mode));
 
   /* When generating a sCOND operation, only positive conditions are
      allowed.  */
@@ -20847,8 +20855,8 @@  rs6000_generate_compare (rtx cmp, machine_mode mode)
 	case UNLT: or1 = UNORDERED;  or2 = LT;  break;
 	default:  gcc_unreachable ();
 	}
-      validate_condition_mode (or1, comp_mode);
-      validate_condition_mode (or2, comp_mode);
+      gcc_assert (validate_condition_mode (or1, comp_mode));
+      gcc_assert (validate_condition_mode (or2, comp_mode));
       or1_rtx = gen_rtx_fmt_ee (or1, SImode, compare_result, const0_rtx);
       or2_rtx = gen_rtx_fmt_ee (or2, SImode, compare_result, const0_rtx);
       compare2_rtx = gen_rtx_COMPARE (CCEQmode,
@@ -20860,7 +20868,7 @@  rs6000_generate_compare (rtx cmp, machine_mode mode)
       code = EQ;
     }
 
-  validate_condition_mode (code, GET_MODE (compare_result));
+  gcc_assert (validate_condition_mode (code, GET_MODE (compare_result)));
 
   return gen_rtx_fmt_ee (code, VOIDmode, compare_result, const0_rtx);
 }
@@ -21368,7 +21376,7 @@  output_cbranch (rtx op, const char *label, int reversed, rtx_insn *insn)
   const char *pred;
   rtx note;
 
-  validate_condition_mode (code, mode);
+  gcc_assert (validate_condition_mode (code, mode));
 
   /* Work out which way this really branches.  We could use
      reverse_condition_maybe_unordered here always but this
@@ -25688,7 +25696,7 @@  rs6000_emit_prologue (void)
       hi = gen_int_mode (toc_restore_insn & ~0xffff, SImode);
       emit_insn (gen_xorsi3 (tmp_reg_si, tmp_reg_si, hi));
       compare_result = gen_rtx_REG (CCUNSmode, CR0_REGNO);
-      validate_condition_mode (EQ, CCUNSmode);
+      gcc_assert (validate_condition_mode (EQ, CCUNSmode));
       lo = gen_int_mode (toc_restore_insn & 0xffff, SImode);
       emit_insn (gen_rtx_SET (compare_result,
 			      gen_rtx_COMPARE (CCUNSmode, tmp_reg_si, lo)));