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

Rohit Arul Raj D rohit.arul.raj.d@nxp.com
Tue Feb 16 05:02:00 GMT 2016


Hello All,

This is related to the following bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60818

Test case:
unsigned int ou;
int jv(void)
{
  unsigned int rg;
  return rg < ou;
}

Command line options used: '-O1' (fails for -O1 and above).

Target: e500v2 (I was able to reproduce with e500mc, e5500 targets as well).

Error message:
0x885f068 validate_condition_mode(rtx_code, machine_mode)
        ../../src_gcc/gcc/config/rs6000/rs6000.c:16389
0x89521d0 branch_comparison_operator(rtx_def*, machine_mode)
        ../../src_gcc/gcc/config/rs6000/predicates.md:1171
0x895229f scc_comparison_operator(rtx_def*, machine_mode)
        ../../src_gcc/gcc/config/rs6000/predicates.md:1221
0x8965eba recog_6
        ../../src_gcc/gcc/config/rs6000/rs6000.md:13910
0x8984e01 recog_20
        ../../src_gcc/gcc/config/rs6000/rs6000.md:343
0x89c6a93 recog(rtx_def*, rtx_def*, int*)
        ../../src_gcc/gcc/config/rs6000/sync.md:128
0x8a2fd4f recog_for_combine
        ../../src_gcc/gcc/combine.c:10888
0x8a1af67 try_combine
        ../../src_gcc/gcc/combine.c:3500
0x8a15ed6 combine_instructions
        ../../src_gcc/gcc/combine.c:1509
0x8a37e71 rest_of_handle_combine
        ../../src_gcc/gcc/combine.c:14204
0x8a37f02 execute
        ../../src_gcc/gcc/combine.c:14247

Analysis so far:

a) The test case passes with '-mno-isel' option.
     The bug description has 2 test cases (comment #4) and both of them pass with this option.

b) The test case fails at this predicate: (as can be seen from the backtrace above) 

;; Return 1 if OP is a comparison operation that is valid for an SCC insn --
;; it must be a positive comparison.
(define_predicate "scc_comparison_operator"
  (and (match_operand 0 "branch_comparison_operator")
       (match_code "eq,lt,gt,ltu,gtu,unordered")))

(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"))))

Corresponding content of "op" which causes the ICE:
gdb) p debug_rtx (op)
(gtu:SI (reg:CC 166)  ---------------------- (operator and mode doesn't match)
    (const_int 0 [0]))
$37 = void

My initial fix was to have signed_scc_comparison_operator and unsigned_scc_comparison_operator but that led to ICE from another stage of combiner pass. So I thought it would be better to fix this at the stage where the conditional mode is being wrongly set.

This is the sequence of steps that the combiner pass does which eventually leads to the ICE:

i) Initial instruction pattern before entering the combiner pass:
(insn 11 10 16 2 (set (reg:SI 165 [ D.2339+-3 ])
        (if_then_else:SI (gtu (reg:CCUNS 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:CCUNS 166)
                (expr_list:REG_EQUAL (gtu:SI (reg:CCUNS 166)
                        (const_int 0 [0]))
                    (nil))))))

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.

File:combine.c (function: combine_instructions)
	  /* Try this insn with each REG_EQUAL note it links back to.  */
	  FOR_EACH_LOG_LINK (links, insn)
	    {
	      rtx set, note;
	      rtx_insn *temp = links->insn;
	....
	...
		{
		  /* Temporarily replace the set's source with the   contents of the REG_EQUAL note.  The insn will
		     be deleted or recognized by try_combine.  */
		  rtx orig = SET_SRC (set);
		  SET_SRC (set) = note;
		  i2mod = temp;
		  i2mod_old_rhs = copy_rtx (orig);
		  i2mod_new_rhs = copy_rtx (note);
		  next = try_combine (insn, i2mod, NULL, NULL,  &new_direct_jump_p,  last_combined_insn); ------------------> fails here.

I have added the required changes here so as to pass the appropriate mode based on the comparison operator. 

diff -Naur gcc-5.3.0/gcc/combine.c gcc-5.3.0-pr60818/gcc/combine.c
--- gcc-5.3.0/gcc/combine.c     2016-02-09 03:33:42.488478524 -0600
+++ gcc-5.3.0-pr60818/gcc/combine.c     2016-02-10 02:28:00.263286834 -0600
@@ -513,6 +513,7 @@
 static void record_truncated_values (rtx *, void *);
 static bool reg_truncated_to_mode (machine_mode, const_rtx);
 static rtx gen_lowpart_or_truncate (machine_mode, rtx);
+static bool can_change_dest_mode (rtx, int, machine_mode);
 
 /* It is not safe to use ordinary gen_lowpart in combine.
@@ -1504,6 +1505,30 @@
                  i2mod = temp;
                  i2mod_old_rhs = copy_rtx (orig);
                  i2mod_new_rhs = copy_rtx (note);
+#ifdef SELECT_CC_MODE
+                 if (comparison_operator (i2mod_new_rhs, VOIDmode))
+                    {
+                      rtx op0 = XEXP (i2mod_new_rhs, 0);
+                      rtx op1 = XEXP (i2mod_new_rhs, 1);
+                      machine_mode compare_mode = GET_MODE (XEXP (i2mod_new_rhs, 0));
+                      machine_mode new_mode = SELECT_CC_MODE (GET_CODE (i2mod_new_rhs),
+                                                             op0, op1);
+                      if (new_mode != compare_mode
+                          && can_change_dest_mode (XEXP (i2mod_new_rhs, 0), 0, new_mode))
+                         {
+                           unsigned int regno = REGNO (op0);
+                           rtx new_dest;
+                           if (regno < FIRST_PSEUDO_REGISTER)
+                             new_dest = gen_rtx_REG (new_mode, regno);
+                           else
+                              {
+                                SUBST_MODE (regno_reg_rtx[regno], new_mode);
+                                new_dest = regno_reg_rtx[regno];
+                              }
+                            SUBST (SET_DEST (i2mod_new_rhs), new_dest);
+                         }
+                    }
+#endif
                  next = try_combine (insn, i2mod, NULL, NULL,
                                      &new_direct_jump_p,
                                      last_combined_insn);

I have run dejaGNU tests with this change on e500mc target and I did find 1 new failure on one of the fortran tests but before I start moving forward wanted to get your comments if this is the right way to fix this issue.

Please let me know your comments.

Regards,
Rohit



More information about the Gcc-patches mailing list