diff mbox series

Fixup IRA debug dump output

Message ID 5dc578d9-49dd-d664-c8bc-ce7a322e9dd3@linux.ibm.com
State New
Headers show
Series Fixup IRA debug dump output | expand

Commit Message

Peter Bergner April 16, 2019, 5:47 p.m. UTC
In looking at the IRA dumps from PR87871, I see that we're missing some \n's
in the output of an allocno's conflicts and we fail to even print the lines
for the hard reg conflicts.  This causes us to start printing the copy and
shuffle info on the same line as the alloc conflicts, like so:

;; a5(r116,l0) conflicts:  cp0:a0(r111)<->a4(r117)@330:move
  cp1:a2(r114)<->a3(r112)@41:shuffle
  cp2:a3(r112)<->a5(r116)@125:shuffle
  pref0:a0(r111)<-hr0@2000
  pref1:a4(r117)<-hr0@660
  pref2:a5(r116)<-hr0@1000
  regions=1, blocks=6, points=10
    allocnos=6 (big 0), copies=3, conflicts=0, ranges=6

The patch below fixes the issue not continuing if the allocno's conflict
array is null and instead guarding the current conflict prints by that
test.  If the conflict array is null, we instead now print out simple
empty conflict info.  This now gives us what we'd expect to see:

;; a5(r116,l0) conflicts:
;;     total conflict hard regs:
;;     conflict hard regs:


  cp0:a0(r111)<->a4(r117)@330:move
  cp1:a2(r114)<->a3(r112)@41:shuffle
  pref0:a0(r111)<-hr0@2000
  pref1:a4(r117)<-hr0@660
  pref2:a5(r116)<-hr0@1000
  regions=1, blocks=6, points=10
    allocnos=6 (big 0), copies=2, conflicts=0, ranges=6
...

Ok for mainline once we hit stage1 again?  Unless people want this now,
given it's only debug output used when -fdump-rtl-ira is used.

Peter

	* ira-conflicts.c (print_allocno_conflicts): Always print something,
	even for allocno's with no conflicts.
	(print_conflicts): Print an extra newline.

Comments

Peter Bergner April 17, 2019, 3:35 p.m. UTC | #1
On 4/16/19 12:47 PM, Peter Bergner wrote:
> The patch below fixes the issue not continuing if the allocno's conflict
> array is null and instead guarding the current conflict prints by that
> test.  If the conflict array is null, we instead now print out simple
> empty conflict info.  This now gives us what we'd expect to see:
> 
> ;; a5(r116,l0) conflicts:
> ;;     total conflict hard regs:
> ;;     conflict hard regs:
> 
> 
>   cp0:a0(r111)<->a4(r117)@330:move


Actually, if we keep the continue, it makes the patch smaller and more
readable.  How about this instead which gives the same output as the
previous patch?

Peter

	* ira-conflicts.c (print_allocno_conflicts): Always print something,
	even for allocno's with no conflicts.
	(print_conflicts): Print an extra newline.

Index: gcc/ira-conflicts.c
===================================================================
--- gcc/ira-conflicts.c	(revision 270331)
+++ gcc/ira-conflicts.c	(working copy)
@@ -633,7 +631,12 @@ print_allocno_conflicts (FILE * file, bo
       ira_object_conflict_iterator oci;
 
       if (OBJECT_CONFLICT_ARRAY (obj) == NULL)
-	continue;
+	{
+	  fprintf (file, "\n;;     total conflict hard regs:\n");
+	  fprintf (file, ";;     conflict hard regs:\n\n");
+	  continue;
+	}
+
       if (n > 1)
 	fprintf (file, "\n;;   subobject %d:", i);
       FOR_EACH_OBJECT_CONFLICT (obj, conflict_obj, oci)
@@ -683,6 +686,7 @@ print_conflicts (FILE *file, bool reg_p)
 
   FOR_EACH_ALLOCNO (a, ai)
     print_allocno_conflicts (file, reg_p, a);
+  putc ('\n', file);
 }
 
 /* Print information about allocno or only regno (if REG_P) conflicts
Jeff Law April 17, 2019, 5:57 p.m. UTC | #2
On 4/17/19 9:35 AM, Peter Bergner wrote:
> On 4/16/19 12:47 PM, Peter Bergner wrote:
>> The patch below fixes the issue not continuing if the allocno's conflict
>> array is null and instead guarding the current conflict prints by that
>> test.  If the conflict array is null, we instead now print out simple
>> empty conflict info.  This now gives us what we'd expect to see:
>>
>> ;; a5(r116,l0) conflicts:
>> ;;     total conflict hard regs:
>> ;;     conflict hard regs:
>>
>>
>>   cp0:a0(r111)<->a4(r117)@330:move
> 
> 
> Actually, if we keep the continue, it makes the patch smaller and more
> readable.  How about this instead which gives the same output as the
> previous patch?
> 
> Peter
> 
> 	* ira-conflicts.c (print_allocno_conflicts): Always print something,
> 	even for allocno's with no conflicts.
> 	(print_conflicts): Print an extra newline.
OK.  And while it's technically not a regression fix, I think this can
safely go in now :-)

jeff
Peter Bergner April 17, 2019, 9:16 p.m. UTC | #3
On 4/17/19 12:57 PM, Jeff Law wrote:
> On 4/17/19 9:35 AM, Peter Bergner wrote:
>> 	* ira-conflicts.c (print_allocno_conflicts): Always print something,
>> 	even for allocno's with no conflicts.
>> 	(print_conflicts): Print an extra newline.
> OK.  And while it's technically not a regression fix, I think this can
> safely go in now :-)


Hi Jeff,

Ok, I committed the patch which is an improvement over the old code.  Thanks!


However, debugging PR87871 some more, I still didn't see p116 conflict with
r0 like Vlad said it did with the new debug output.  Not surprising, since
the patch only affected adding missing \n's to the output.  So I dumped the
OBJECT_TOTAL_CONFLICT_HARD_REGS() output for p116 and sure enough, it does
mention r0.  I then called print_allocno_conflicts() by hand and it still
didn't output r0 as a conflicting hard reg.  Stepping through the debugger,
I see that the:

  if (OBJECT_CONFLICT_ARRAY (obj) != NULL)
    {
      fprintf (file, "\n;;     total conflict hard regs:\n");
      fprintf (file, ";;     conflict hard regs:\n\n");
      continue;
    }

...is actually incorrect.  The "if" test only says we don't have any
conflicts with any other allocnos/pseudos.  It doesn't tell us whether we
have any hard register conflicts or not, so we really shouldn't do a continue
here.  Instead, we should guard the code that outputs the allocno conflicts
and then fall down to the hard reg conflict prints, that should also be
suitably guarded.  With the patch below, we now see the missing r0 conflict
Vlad said was there.

  ;; a5(r116,l0) conflicts:
  ;;     total conflict hard regs: 0
  ;;     conflict hard regs:


    cp0:a0(r111)<->a4(r117)@330:move
    cp1:a2(r114)<->a3(r112)@41:shuffle
    ...

Note, I still don't understand why p116 conflicts with r0, but that is
orthogonal to actually printing out the conflict sets as they exist.

Is this ok as well?  ...and I'm sorry for not noticing this issue before.

Peter

	* ira-conflicts.c (print_allocno_conflicts): Print the hard register
	conflicts, even if there are no allocno conflicts.

Index: gcc/ira-conflicts.c
===================================================================
--- gcc/ira-conflicts.c	(revision 270420)
+++ gcc/ira-conflicts.c	(working copy)
@@ -632,47 +632,58 @@ print_allocno_conflicts (FILE * file, bo
       ira_object_t conflict_obj;
       ira_object_conflict_iterator oci;
 
-      if (OBJECT_CONFLICT_ARRAY (obj) == NULL)
+      if (OBJECT_CONFLICT_ARRAY (obj) != NULL)
 	{
-	  fprintf (file, "\n;;     total conflict hard regs:\n");
-	  fprintf (file, ";;     conflict hard regs:\n\n");
-	  continue;
-	}
-
-      if (n > 1)
-	fprintf (file, "\n;;   subobject %d:", i);
-      FOR_EACH_OBJECT_CONFLICT (obj, conflict_obj, oci)
-	{
-	  ira_allocno_t conflict_a = OBJECT_ALLOCNO (conflict_obj);
-	  if (reg_p)
-	    fprintf (file, " r%d,", ALLOCNO_REGNO (conflict_a));
-	  else
+	  if (n > 1)
+	    fprintf (file, "\n;;   subobject %d:", i);
+	  FOR_EACH_OBJECT_CONFLICT (obj, conflict_obj, oci)
 	    {
-	      fprintf (file, " a%d(r%d", ALLOCNO_NUM (conflict_a),
-		       ALLOCNO_REGNO (conflict_a));
-	      if (ALLOCNO_NUM_OBJECTS (conflict_a) > 1)
-		fprintf (file, ",w%d", OBJECT_SUBWORD (conflict_obj));
-	      if ((bb = ALLOCNO_LOOP_TREE_NODE (conflict_a)->bb) != NULL)
-		fprintf (file, ",b%d", bb->index);
+	      ira_allocno_t conflict_a = OBJECT_ALLOCNO (conflict_obj);
+	      if (reg_p)
+		fprintf (file, " r%d,", ALLOCNO_REGNO (conflict_a));
 	      else
-		fprintf (file, ",l%d",
-			 ALLOCNO_LOOP_TREE_NODE (conflict_a)->loop_num);
-	      putc (')', file);
+		{
+		  fprintf (file, " a%d(r%d", ALLOCNO_NUM (conflict_a),
+			   ALLOCNO_REGNO (conflict_a));
+		  if (ALLOCNO_NUM_OBJECTS (conflict_a) > 1)
+		    fprintf (file, ",w%d", OBJECT_SUBWORD (conflict_obj));
+		  if ((bb = ALLOCNO_LOOP_TREE_NODE (conflict_a)->bb) != NULL)
+		    fprintf (file, ",b%d", bb->index);
+		  else
+		    fprintf (file, ",l%d",
+			     ALLOCNO_LOOP_TREE_NODE (conflict_a)->loop_num);
+		  putc (')', file);
+		}
 	    }
 	}
-      COPY_HARD_REG_SET (conflicting_hard_regs, OBJECT_TOTAL_CONFLICT_HARD_REGS (obj));
-      AND_COMPL_HARD_REG_SET (conflicting_hard_regs, ira_no_alloc_regs);
-      AND_HARD_REG_SET (conflicting_hard_regs,
-			reg_class_contents[ALLOCNO_CLASS (a)]);
-      print_hard_reg_set (file, "\n;;     total conflict hard regs:",
-			  conflicting_hard_regs);
-
-      COPY_HARD_REG_SET (conflicting_hard_regs, OBJECT_CONFLICT_HARD_REGS (obj));
-      AND_COMPL_HARD_REG_SET (conflicting_hard_regs, ira_no_alloc_regs);
-      AND_HARD_REG_SET (conflicting_hard_regs,
-			reg_class_contents[ALLOCNO_CLASS (a)]);
-      print_hard_reg_set (file, ";;     conflict hard regs:",
-			  conflicting_hard_regs);
+
+      if (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj) != NULL)
+	{
+	  COPY_HARD_REG_SET (conflicting_hard_regs,
+			     OBJECT_TOTAL_CONFLICT_HARD_REGS (obj));
+	  AND_COMPL_HARD_REG_SET (conflicting_hard_regs, ira_no_alloc_regs);
+	  AND_HARD_REG_SET (conflicting_hard_regs,
+			    reg_class_contents[ALLOCNO_CLASS (a)]);
+	  print_hard_reg_set (file, "\n;;     total conflict hard regs:",
+			      conflicting_hard_regs);
+	}
+      else
+	fprintf (file, "\n;;     total conflict hard regs:\n");
+
+
+      if (OBJECT_CONFLICT_HARD_REGS (obj) != NULL)
+	{
+	  COPY_HARD_REG_SET (conflicting_hard_regs,
+			     OBJECT_CONFLICT_HARD_REGS (obj));
+	  AND_COMPL_HARD_REG_SET (conflicting_hard_regs, ira_no_alloc_regs);
+	  AND_HARD_REG_SET (conflicting_hard_regs,
+			    reg_class_contents[ALLOCNO_CLASS (a)]);
+	  print_hard_reg_set (file, ";;     conflict hard regs:",
+			      conflicting_hard_regs);
+	}
+      else
+	fprintf (file, ";;     conflict hard regs:\n");
+
       putc ('\n', file);
     }
diff mbox series

Patch

Index: gcc/ira-conflicts.c
===================================================================
--- gcc/ira-conflicts.c	(revision 270331)
+++ gcc/ira-conflicts.c	(working copy)
@@ -632,45 +630,50 @@  print_allocno_conflicts (FILE * file, bo
       ira_object_t conflict_obj;
       ira_object_conflict_iterator oci;
 
-      if (OBJECT_CONFLICT_ARRAY (obj) == NULL)
-	continue;
-      if (n > 1)
-	fprintf (file, "\n;;   subobject %d:", i);
-      FOR_EACH_OBJECT_CONFLICT (obj, conflict_obj, oci)
+      if (OBJECT_CONFLICT_ARRAY (obj) != NULL)
 	{
-	  ira_allocno_t conflict_a = OBJECT_ALLOCNO (conflict_obj);
-	  if (reg_p)
-	    fprintf (file, " r%d,", ALLOCNO_REGNO (conflict_a));
-	  else
+	  if (n > 1)
+	    fprintf (file, "\n;;   subobject %d:", i);
+	  FOR_EACH_OBJECT_CONFLICT (obj, conflict_obj, oci)
 	    {
-	      fprintf (file, " a%d(r%d", ALLOCNO_NUM (conflict_a),
-		       ALLOCNO_REGNO (conflict_a));
-	      if (ALLOCNO_NUM_OBJECTS (conflict_a) > 1)
-		fprintf (file, ",w%d", OBJECT_SUBWORD (conflict_obj));
-	      if ((bb = ALLOCNO_LOOP_TREE_NODE (conflict_a)->bb) != NULL)
-		fprintf (file, ",b%d", bb->index);
+	      ira_allocno_t conflict_a = OBJECT_ALLOCNO (conflict_obj);
+	      if (reg_p)
+		fprintf (file, " r%d,", ALLOCNO_REGNO (conflict_a));
 	      else
-		fprintf (file, ",l%d",
-			 ALLOCNO_LOOP_TREE_NODE (conflict_a)->loop_num);
-	      putc (')', file);
+		{
+		  fprintf (file, " a%d(r%d", ALLOCNO_NUM (conflict_a),
+			   ALLOCNO_REGNO (conflict_a));
+		  if (ALLOCNO_NUM_OBJECTS (conflict_a) > 1)
+		    fprintf (file, ",w%d", OBJECT_SUBWORD (conflict_obj));
+		  if ((bb = ALLOCNO_LOOP_TREE_NODE (conflict_a)->bb) != NULL)
+		    fprintf (file, ",b%d", bb->index);
+		  else
+		    fprintf (file, ",l%d",
+			     ALLOCNO_LOOP_TREE_NODE (conflict_a)->loop_num);
+		  putc (')', file);
+		}
 	    }
+	  COPY_HARD_REG_SET (conflicting_hard_regs, OBJECT_TOTAL_CONFLICT_HARD_REGS (obj));
+	  AND_COMPL_HARD_REG_SET (conflicting_hard_regs, ira_no_alloc_regs);
+	  AND_HARD_REG_SET (conflicting_hard_regs,
+			    reg_class_contents[ALLOCNO_CLASS (a)]);
+	  print_hard_reg_set (file, "\n;;     total conflict hard regs:",
+			      conflicting_hard_regs);
+
+	  COPY_HARD_REG_SET (conflicting_hard_regs, OBJECT_CONFLICT_HARD_REGS (obj));
+	  AND_COMPL_HARD_REG_SET (conflicting_hard_regs, ira_no_alloc_regs);
+	  AND_HARD_REG_SET (conflicting_hard_regs,
+			    reg_class_contents[ALLOCNO_CLASS (a)]);
+	  print_hard_reg_set (file, ";;     conflict hard regs:",
+			      conflicting_hard_regs);
+	}
+      else
+	{
+	  fprintf (file, "\n;;     total conflict hard regs:\n");
+	  fprintf (file, ";;     conflict hard regs:\n");
 	}
-      COPY_HARD_REG_SET (conflicting_hard_regs, OBJECT_TOTAL_CONFLICT_HARD_REGS (obj));
-      AND_COMPL_HARD_REG_SET (conflicting_hard_regs, ira_no_alloc_regs);
-      AND_HARD_REG_SET (conflicting_hard_regs,
-			reg_class_contents[ALLOCNO_CLASS (a)]);
-      print_hard_reg_set (file, "\n;;     total conflict hard regs:",
-			  conflicting_hard_regs);
-
-      COPY_HARD_REG_SET (conflicting_hard_regs, OBJECT_CONFLICT_HARD_REGS (obj));
-      AND_COMPL_HARD_REG_SET (conflicting_hard_regs, ira_no_alloc_regs);
-      AND_HARD_REG_SET (conflicting_hard_regs,
-			reg_class_contents[ALLOCNO_CLASS (a)]);
-      print_hard_reg_set (file, ";;     conflict hard regs:",
-			  conflicting_hard_regs);
       putc ('\n', file);
     }
-
 }
 
 /* Print information about allocno or only regno (if REG_P) conflicts
@@ -683,6 +686,7 @@  print_conflicts (FILE *file, bool reg_p)
 
   FOR_EACH_ALLOCNO (a, ai)
     print_allocno_conflicts (file, reg_p, a);
+  putc ('\n', file);
 }
 
 /* Print information about allocno or only regno (if REG_P) conflicts