This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Better fix for the x86_64 -mcmodel=large ICEs (PR target/82145)


Hi!

The testcase below (and others) still ICEs with my PR81766 fix.
If there is a cfg cleanup in between ix86_init_pic_reg (during RA)
and postreload, the label which my fix moved to the right spot is
turned into NOTE_INSN_DELETED_LABEL note and moved back where it
originally used to be emitted.

The bug is actually in generic code.
cselib.c has code to handle LABEL_REF properly during hashing, by not
hashing the instructions/notes that are operand thereof, but just the
label number.  Postreload though calls cselib_lookup directly on the
operands of an instruction, and it is very common in many backends to have
(label_ref (match_operand ...)) in many patterns, and thus cselib
doesn't use the LABEL_REF handling in that case.
To avoid crashing postreload.c has:
       /* cselib blows up on CODE_LABELs.  Trying to fix that doesn't seem
          right, so avoid the problem here.  Likewise if we have a constant
          and the insn pattern doesn't tell us the mode we need.  */
       if (LABEL_P (recog_data.operand[i])
           || (CONSTANT_P (recog_data.operand[i])
               && recog_data.operand_mode[i] == VOIDmode))
         continue;
- we won't look up anything useful for those operands anyway.
The problem is that a valid LABEL_REF operand doesn't have to be only
a CODE_LABEL, it can be a NOTE with NOTE_INSN_DELETED_LABEL if all we need
is the label's address and nothing else.

So, the following patch handles NOTE_INSN_DELETED_LABEL like CODE_LABEL
in postreload.c.

Once that is done, my earlier PR81766 fix can be effectively reverted
and instead the CODE_LABEL can be immediately turned into
NOTE_INSN_DELETED_LABEL, something that a cfgcleanup would do.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-09-13  Jakub Jelinek  <jakub@redhat.com>

	PR target/82145
	* postreload.c (reload_cse_simplify_operands): Skip
	NOTE_INSN_DELETED_LABEL similarly to skipping CODE_LABEL.
	* config/i386/i386.c (ix86_init_large_pic_reg): Revert 2017-09-01
	changes.  Turn CODE_LABEL into NOTE_INSN_DELETED_LABEL immediately.
	(ix86_init_pic_reg): Revert 2017-09-01 changes.

	* gcc.target/i386/pr82145.c: New test.

--- gcc/config/i386/i386.c.jj	2017-09-08 09:13:59.000000000 +0200
+++ gcc/config/i386/i386.c	2017-09-11 14:48:50.532094255 +0200
@@ -8885,7 +8885,7 @@ ix86_use_pseudo_pic_reg (void)
 
 /* Initialize large model PIC register.  */
 
-static rtx_code_label *
+static void
 ix86_init_large_pic_reg (unsigned int tmp_regno)
 {
   rtx_code_label *label;
@@ -8902,7 +8902,10 @@ ix86_init_large_pic_reg (unsigned int tm
   emit_insn (gen_set_got_offset_rex64 (tmp_reg, label));
   emit_insn (ix86_gen_add3 (pic_offset_table_rtx,
 			    pic_offset_table_rtx, tmp_reg));
-  return label;
+  const char *name = LABEL_NAME (label);
+  PUT_CODE (label, NOTE);
+  NOTE_KIND (label) = NOTE_INSN_DELETED_LABEL;
+  NOTE_DELETED_LABEL_NAME (label) = name;
 }
 
 /* Create and initialize PIC register if required.  */
@@ -8911,7 +8914,6 @@ ix86_init_pic_reg (void)
 {
   edge entry_edge;
   rtx_insn *seq;
-  rtx_code_label *label = NULL;
 
   if (!ix86_use_pseudo_pic_reg ())
     return;
@@ -8921,7 +8923,7 @@ ix86_init_pic_reg (void)
   if (TARGET_64BIT)
     {
       if (ix86_cmodel == CM_LARGE_PIC)
-	label = ix86_init_large_pic_reg (R11_REG);
+	ix86_init_large_pic_reg (R11_REG);
       else
 	emit_insn (gen_set_got_rex64 (pic_offset_table_rtx));
     }
@@ -8945,22 +8947,6 @@ ix86_init_pic_reg (void)
   entry_edge = single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun));
   insert_insn_on_edge (seq, entry_edge);
   commit_one_edge_insertion (entry_edge);
-
-  if (label)
-    {
-      basic_block bb = BLOCK_FOR_INSN (label);
-      rtx_insn *bb_note = PREV_INSN (label);
-      /* If the note preceding the label starts a basic block, and the
-	 label is a member of the same basic block, interchange the two.  */
-      if (bb_note != NULL_RTX
-	  && NOTE_INSN_BASIC_BLOCK_P (bb_note)
-	  && bb != NULL
-	  && bb == BLOCK_FOR_INSN (bb_note))
-	{
-	  reorder_insns_nobb (bb_note, bb_note, label);
-	  BB_HEAD (bb) = label;
-	}
-    }
 }
 
 /* Initialize a variable CUM of type CUMULATIVE_ARGS
--- gcc/postreload.c.jj	2017-09-08 09:13:54.000000000 +0200
+++ gcc/postreload.c	2017-09-11 14:49:04.977945563 +0200
@@ -409,9 +409,12 @@ reload_cse_simplify_operands (rtx_insn *
       CLEAR_HARD_REG_SET (equiv_regs[i]);
 
       /* cselib blows up on CODE_LABELs.  Trying to fix that doesn't seem
-	 right, so avoid the problem here.  Likewise if we have a constant
-         and the insn pattern doesn't tell us the mode we need.  */
+	 right, so avoid the problem here.  Similarly NOTE_INSN_DELETED_LABEL.
+	 Likewise if we have a constant and the insn pattern doesn't tell us
+	 the mode we need.  */
       if (LABEL_P (recog_data.operand[i])
+	  || (NOTE_P (recog_data.operand[i])
+	      && NOTE_KIND (recog_data.operand[i]) == NOTE_INSN_DELETED_LABEL)
 	  || (CONSTANT_P (recog_data.operand[i])
 	      && recog_data.operand_mode[i] == VOIDmode))
 	continue;
--- gcc/testsuite/gcc.target/i386/pr82145.c.jj	2017-09-11 14:46:08.612760951 +0200
+++ gcc/testsuite/gcc.target/i386/pr82145.c	2017-09-11 14:47:22.090004622 +0200
@@ -0,0 +1,12 @@
+/* PR target/82145 */
+/* { dg-do compile { target { pie && lp64 } } } */
+/* { dg-options "-O2 -fpie -mcmodel=large -march=haswell" } */
+
+int l;
+
+int
+main ()
+{
+  l++;
+  return 0;
+}

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]