Re: [x86, PATCH 2/2] Enabling of the new Intel microarchitecture Silvermont

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, May 31, 2013 at 04:56:35PM +0400, Igor Zamyatin wrote:
> Like this?

Sorry for nitpicking, but there are various formatting issues.

+  if (ix86_tune == PROCESSOR_SLM)
+    {
+      if (has_scale)
+        return true;
+      if (split_cost < 1)
+        return false;
+      if (regno0 == regno1 || regno0 == regno2)
+        return false;
+      return true;

8 spaces instead of tab (several times)?

+  return !ix86_lea_outperforms (insn, regno0, regno1, regno2, split_cost,
+                                parts.scale > 1);
 }

Likewise.
 
+static bool
+exact_dependency_1 (rtx addr, rtx insn)
+{
+  enum rtx_code code;
+  const char *format_ptr;
+  int i, j;
+
+  code = GET_CODE (insn);
+  switch (code)
+    {
+    case MEM:
+      if (rtx_equal_p (addr, insn))
+        return true;
+      break;
+    case REG:
+    CASE_CONST_ANY:
+    case SYMBOL_REF:
+    case CODE_LABEL:
+    case PC:
+    case CC0:
+    case EXPR_LIST:
+      return false;
+    default:
+      break;
+    }

Likewise.
+
+  format_ptr = GET_RTX_FORMAT (code);
+  for (i = 0; i < GET_RTX_LENGTH (code); i++)
+    {
+      switch (*format_ptr++)
+       {
+       case 'e':
+         if (exact_dependency_1 (addr, XEXP (insn, i)))
+           return true;
+         break;
+       case 'E':
+         for (j = 0; j < XVECLEN (insn, i); j++)
+           if (exact_dependency_1 (addr, XVECEXP (insn, i, j)))
+             return true;
+         break;
+       }

Likewise.

+    case PROCESSOR_SLM:
+      if (!reload_completed)
+        return cost;

Likewise.
+
+      /* Increase cost of integer loads.  */
+      memory = get_attr_memory (dep_insn);
+      if (memory == MEMORY_LOAD || memory == MEMORY_BOTH)
+        {
+          enum attr_unit unit = get_attr_unit (dep_insn);
+          if (unit == UNIT_INTEGER && cost == 1)
+            {
+              if (memory == MEMORY_LOAD)
+                cost = 3;
+              else
+                {
+                  /* Increase cost of ld/st for short int types only
+                     because of store forwarding issue.  */
+                  rtx set = single_set (dep_insn);
+                  if (set && (GET_MODE (SET_DEST (set)) == QImode
+                              || GET_MODE (SET_DEST (set)) == HImode))
+                    {
+                      /* Increase cost of store/load insn if exact
+                         dependence exists and it is load insn.  */
+                      enum attr_memory insn_memory = get_attr_memory (insn);
+                      if (insn_memory == MEMORY_LOAD
+                          && exact_store_load_dependency (dep_insn, insn))
+                        cost = 3;
+                    }
+                }
+            }
+        }

Likewise lots of times (and several times more in the patch).

@@ -24565,47 +24685,32 @@ ia32_multipass_dfa_lookahead (void)
    execution. It is applied if
    (1) IMUL instruction is on the top of list;
    (2) There exists the only producer of independent IMUL instruction in
-       ready list;
-   (3) Put found producer on the top of ready list.
-   Returns issue rate.  */
-
+       ready list.
+   Return index of IMUL producer if it was found and -1 otherwise.  */
 static int
-ix86_sched_reorder(FILE *dump, int sched_verbose, rtx *ready, int *pn_ready,
-                   int clock_var ATTRIBUTE_UNUSED)
+do_reoder_for_imul(rtx *ready, int n_ready)

do_reorder_for_imul ?  Also, missing space between imul and (.

+/* Try to find the best candidate on the top of ready list if two insns
+   have the same priority - candidate is best if its dependees were
+   scheduled earlier. Applied for Silvermont only.
+   Return true if top 2 insns must be interchanged.  */
+static bool
+swap_top_of_ready_list(rtx *ready, int n_ready)

Missing space before (.

+/* Perform possible reodering of ready list for Atom/Silvermont only.
+   Return issue rate.  */
+static int
+ix86_sched_reorder(FILE *dump, int sched_verbose, rtx *ready, int *pn_ready,
+                   int clock_var)

Likewise.
+{
+  int issue_rate = -1;
+  int n_ready = *pn_ready;
+  int i;
+  rtx insn;
+  int index = -1;
+
+  /* Set up issue rate.  */
+  issue_rate = ix86_issue_rate();

Likewise.

+        fprintf(dump, ";;\tatom sched_reorder: put %d insn on top\n",
+                INSN_UID (ready[index]));

Likewise.

+      /* Put IMUL producer (ready[index]) at the top of ready list.  */
+      insn= ready[index];

Missing space before =.

+      if (sched_verbose > 1)
+        fprintf(dump, ";;\tslm sched_reorder: swap %d and %d insns\n",
+                INSN_UID (ready[n_ready - 1]), INSN_UID (ready[n_ready - 2]));

Missing space before (.

	Jakub