Dataflow branch review #4

[Ian Lance Taylor <iant at google dot com>]

In config/ia64/ia64.c:

+  int r[number_of_ia64_frame_regs];
+#if 0
   int reg_fp;			/* register for fp.  */
   int reg_save_b0;		/* save register for b0.  */
   int reg_save_pr;		/* save register for prs.  */
@@ -143,6 +159,7 @@ struct ia64_frame_info
   int reg_save_ar_unat;		/* save register for ar.unat.  */
   int reg_save_ar_lc;		/* save register for ar.lc.  */
   int reg_save_gp;		/* save register for gp.  */
+#endif

Just remove the lines between #if 0 and #endif.


In ia64_compute_frame_size:

-      current_frame_info.reg_save_b0 = find_gr_spill (1);
-      if (current_frame_info.reg_save_b0 == 0)
+      current_frame_info.r[reg_save_b0] = find_gr_spill (reg_save_b0, 1);
+      /* reg_emitted (reg_save_b0); */
+      if (current_frame_info.r[reg_save_b0] == 0)

I'm not sure what the comment means.

-      current_frame_info.reg_save_ar_unat = find_gr_spill (spill_size == 0);
-      if (current_frame_info.reg_save_ar_unat == 0)
+      current_frame_info.r[reg_save_ar_unat] 
+        = find_gr_spill (reg_save_ar_unat, spill_size == 0);
+      /* reg_emitted (reg_save_ar_unat); */
+      if (current_frame_info.r[reg_save_ar_unat] == 0)

Again.

-      current_frame_info.reg_save_ar_lc = find_gr_spill (spill_size == 0);
-      if (current_frame_info.reg_save_ar_lc == 0)
+      current_frame_info.r[reg_save_ar_lc] 
+        = find_gr_spill (reg_save_ar_lc, spill_size == 0);
+      /* reg_emitted (reg_save_ar_lc); */
+      if (current_frame_info.r[reg_save_ar_lc] == 0)

And again.


In ia64_expand_prologue:

+  if (dump_file) 
+    {
+#define PRINTREG(a) if (current_frame_info.r[a]) fprintf(dump_file, "%s = %d\n", #a, current_frame_info.r[a])
+      PRINTREG(reg_fp);
+      PRINTREG(reg_save_b0);
+      PRINTREG(reg_save_pr);
+      PRINTREG(reg_save_ar_pfs);
+      PRINTREG(reg_save_ar_unat);
+      PRINTREG(reg_save_ar_lc);
+      PRINTREG(reg_save_gp);
+#undef PRINTREG
+    }

I think this debugging info should have some sort of header.  It looks
like it will be kind of obscure without one.


In config/ia64/ia64.h:

+extern void ia64_init_expanders (void);

Move this declaration into ia64-protos.h instead.



In cselib.c:

In cselib_expand_value_rtx:

+/* Forward substitute and expand an expression out to it's roots.

Typo: "it's" => "its".

+	      /* The several thing that we are not willing to do (this
+		 is requirement of dse and if others potiential uses
+		 need this function we should add a parm to control
+		 it) is that we will not substitute the
+		 STACK_POINTER_REGNUM, FRAME_POINTER or the
+		 HARD_FRAME_POINTER.

Something wrong in the first sentence.

+		 HARD_FRAME_POINTER then you loose the opportunity to

Typo: "loose" => "lose".

+    case CONST:
+      /* CONST can be shared if it contains a SYMBOL_REF.  If it contains
+	 a LABEL_REF, it isn't sharable.  */
+      if (GET_CODE (XEXP (orig, 0)) == PLUS
+	  && GET_CODE (XEXP (XEXP (orig, 0), 0)) == SYMBOL_REF
+	  && GET_CODE (XEXP (XEXP (orig, 0), 1)) == CONST_INT)
+	return orig;
+      break;

This is ugly.  Here and elsewhere you are building in knowledge about
what is sharable.  This will be a maintenance issue in the future.
Perhaps it would be better if you added a helper function in
emit-rtl.c to indicate whether an rtx should be copied, and add code
to, e.g., copy_insn_1 to call it as well.


In dbgcnt.c:

+   while (comma)
+     {
+       colon = strchr (comma + 1, ':');
+       if (colon == NULL || !(colon[1] >= '0' && colon[1] <= '9'))
+         return;
+       dbg_cnt_set_limit_by_name (comma + 1, colon - (comma + 1), atoi (colon + 1));
+       comma = strchr (colon + 1, ',');
+     }

It seems like if you make a mistake writing the parameter you will get
no warning.  I think that is a bad idea, even for debugging code.
There should be a warning for invalid option syntax or for a
nonexistent parameter.


In dbgcnt.def:

+   By default, all limits are UINT_MAX.
+   Since debug count is unsigned int, <= UINT_MAX returns true always.
+   i.e.  dbg_cnt() returns true always regardless of the counter value
+   (although it still counts the event).
+   Use -fdbg-cnt=counter1:N,counter2:M,...
+   which sets the limit for counter1 to N, and the limit for counter2 to M, etc.
+   e.g. setting a limit to zero will make dbg_cnt () return false *always*.

This needs to be documented in doc/invoke.texi.  It's probably not
necessary to document each of the support debugging names.


In rtl.texi:

+the schedulars and by combine.  This is a deprecated data structure.

Typo: "schedulars" => "schedulers".


In dse.c:

+   are the singletons.  Rather that analyze the addresses of the

Typo: "that" => "than".


+/* This structure holds information about a candidate store.  */
+struct store_info {

Put the '{' at the start of the next line, here and in a number of
other struct definitions later on in the file.

+static void
+step0 (void)

Let's name all these functions dse_step0, and so on, so that it is
easier to set a breakpoint on them when debugging.