Patchwork Fix decompress of min_issue_delay

login
register
mail settings
Submitter Jie Zhang
Date June 19, 2010, 2 a.m.
Message ID <4C1C24AB.9010401@codesourcery.com>
Download mbox | patch
Permalink /patch/56235/
State New
Headers show

Comments

Jie Zhang - June 19, 2010, 2 a.m.
When debugging a scheduling problem of GCC, I found min_issue_delay was 
not decompressed correctly, which caused weird things, like modifying a 
reservation changes scheduling of a sequence of instructions but none of 
those instructions uses that reservation.

In output_min_issue_delay_table, the table is compressed with

      for (i = 0; i < min_issue_delay_len; i++)
        {
          size_t ci = i / cfactor;
          vect_el_t x = VEC_index (vect_el_t, min_issue_delay_vect, i);
          vect_el_t cx = VEC_index (vect_el_t, 
compressed_min_issue_delay_vect, ci);

A==>  cx |= x << (8 - (i % cfactor + 1) * (8 / cfactor));
          VEC_replace (vect_el_t, compressed_min_issue_delay_vect, ci, cx);
        }

But, take Cortex-A8 as an example, the table is decompressed in 
generated internal_min_issue_delay as

          temp = cortex_a8_min_issue_delay [(cortex_a8_translate 
[insn_code] + chip->cortex_a8_automaton_state * 9) / 4];
B==>  temp = (temp >> (8 - (cortex_a8_translate [insn_code] % 4 + 1) * 
2)) & 3;
          res = temp;

Note it's possible that line B does not decompress out the same value 
which line A put into the table. The 'i' in line A is same as 
"(cortex_a8_translate [insn_code] + chip->cortex_a8_automaton_state * 
9)". But line B uses "cortex_a8_translate [insn_code]".

This patch should fix it. Bootstrapped and regression tested on x86-64.

Is it OK?


--
Jie Zhang
CodeSourcery
Jie Zhang - June 28, 2010, 2:29 a.m.
Hi Vladimir,

Could you please take a look at this patch? Thanks!

Jie

On 06/19/2010 10:00 AM, Jie Zhang wrote:
> When debugging a scheduling problem of GCC, I found min_issue_delay was
> not decompressed correctly, which caused weird things, like modifying a
> reservation changes scheduling of a sequence of instructions but none of
> those instructions uses that reservation.
>
> In output_min_issue_delay_table, the table is compressed with
>
> for (i = 0; i < min_issue_delay_len; i++)
> {
> size_t ci = i / cfactor;
> vect_el_t x = VEC_index (vect_el_t, min_issue_delay_vect, i);
> vect_el_t cx = VEC_index (vect_el_t, compressed_min_issue_delay_vect, ci);
>
> A==> cx |= x << (8 - (i % cfactor + 1) * (8 / cfactor));
> VEC_replace (vect_el_t, compressed_min_issue_delay_vect, ci, cx);
> }
>
> But, take Cortex-A8 as an example, the table is decompressed in
> generated internal_min_issue_delay as
>
> temp = cortex_a8_min_issue_delay [(cortex_a8_translate [insn_code] +
> chip->cortex_a8_automaton_state * 9) / 4];
> B==> temp = (temp >> (8 - (cortex_a8_translate [insn_code] % 4 + 1) *
> 2)) & 3;
> res = temp;
>
> Note it's possible that line B does not decompress out the same value
> which line A put into the table. The 'i' in line A is same as
> "(cortex_a8_translate [insn_code] + chip->cortex_a8_automaton_state *
> 9)". But line B uses "cortex_a8_translate [insn_code]".
>
> This patch should fix it. Bootstrapped and regression tested on x86-64.
>
> Is it OK?
>
>
> --
> Jie Zhang
> CodeSourcery
>
>
>
>
>

Patch


	* genautomata.c (output_automata_list_min_issue_delay_code):
	Correctly decompress min_issue_delay.

Index: genautomata.c
===================================================================
--- genautomata.c	(revision 160987)
+++ genautomata.c	(working copy)
@@ -7854,12 +7854,15 @@  output_automata_list_min_issue_delay_cod
 	{
 	  fprintf (output_file, ") / %d];\n",
 		   automaton->min_issue_delay_table_compression_factor);
-	  fprintf (output_file, "      %s = (%s >> (8 - (",
+	  fprintf (output_file, "      %s = (%s >> (8 - ((",
 		   TEMPORARY_VARIABLE_NAME, TEMPORARY_VARIABLE_NAME);
 	  output_translate_vect_name (output_file, automaton);
+	  fprintf (output_file, " [%s] + ", INTERNAL_INSN_CODE_NAME);
+	  fprintf (output_file, "%s->", CHIP_PARAMETER_NAME);
+	  output_chip_member_name (output_file, automaton);
+	  fprintf (output_file, " * %d)", automaton->insn_equiv_classes_num);
 	  fprintf
-	    (output_file, " [%s] %% %d + 1) * %d)) & %d;\n",
-	     INTERNAL_INSN_CODE_NAME,
+	    (output_file, " %% %d + 1) * %d)) & %d;\n",
 	     automaton->min_issue_delay_table_compression_factor,
 	     8 / automaton->min_issue_delay_table_compression_factor,
 	     (1 << (8 / automaton->min_issue_delay_table_compression_factor))