openpower-cores / a2i

Other
243 stars 40 forks source link

The below water remark in iuq_ib_buff.vhdl #23

Closed Sky-HChina closed 3 years ago

Sky-HChina commented 3 years ago

In iuq_ib_buff.vhdl, the "ib_ic_below_water" signal is set to 1 when "buffer4_valid_l2=1'b0" and the "ib_ic_empty" signal is set to 1 when "buffer1_valid_l2=1'b0".

ib_ic_empty <= not (buffer1_valid_l2 or stall_l2(0));
ib_ic_below_water <= (not buffer4_valid_l2) or (not buffer5_valid_l2 and not stall_l2(0));

Then, in iuq_ic_select.vhdl file, "low_mask_d" signal is set when "ib_ic_below_water" is 1 and "high_mask_d" is set when "ib_ic_empty" is 1. I am confused here.

According to the A2I manual, "There are two watermarks within the instruction buffer that determine a thread’s priority level for fetches that are empty and half-empty, a halfempty level gives the thread a low-priority fetch request."

If "ib_ic_below_water" represets the half-empty, I think it should be set to 1 when "buffer4_valid_l2" is 1.

The current code seems to me "ib_ic_empty" and "ib_ic_below_water" will both be set to 1 if the insturction buffer is totally empty. This clearly has conflication since low_mask_d and high_mask_d will be set to 1 at the same time.

openpowerwtf commented 3 years ago

It seems true that empty/below_water can be active together, unless there's an extra condition to force them to be mutually exclusive. This table implies low_mask is dont-care if any high_mask bits are set, when creating the valids (next_low_valid=0 if any high_mask bit is set).

> ---- Round robin q for selecting the next thread
> --?TABLE select_next LISTING(final) OPTIMIZE PARMS(ON-SET, OFF-SET);
> --*INPUTS*==============================*OUTPUTS*===============*
> --|                                     |                       |
> --| iu0_last_tid_high_l2                |  next_high_valid      |
> --| |     high_mask_l2                  |  | next_low_valid     | 
> --| |     |                             |  | |  next_tid        |
> --| |     |       iu0_last_tid_low_l2   |  | |  |               |
> --| |     |       |     low_mask_l2     |  | |  |               |
> --| |     |       |     |               |  | |  |               |
> --| |     |       |     |               |  | |  |               |
> --| 0123  0123    0123  0123            |  | |  0123            |
> --*TYPE*================================+=======================+
> --| PPPP  PPPP    PPPP  PPPP            |  P P  PPPP            |
> --*OPTIMIZE*--------------------------->|  A A  BBBB            |
> --*TERMS*===============================+=======================+
> --| 1000  -1--    ----  ----            |  1 0  0100            |
> --| 1000  -01-    ----  ----            |  1 0  0010            |
> --| 1000  -001    ----  ----            |  1 0  0001            |
> --| 1000  1000    ----  ----            |  1 0  1000            |
> --| 0100  --1-    ----  ----            |  1 0  0010            |
> --| 0100  --01    ----  ----            |  1 0  0001            |
> --| 0100  1-00    ----  ----            |  1 0  1000            |
> --| 0100  0100    ----  ----            |  1 0  0100            |
> --| 0010  ---1    ----  ----            |  1 0  0001            |
> --| 0010  1--0    ----  ----            |  1 0  1000            |
> --| 0010  01-0    ----  ----            |  1 0  0100            |
> --| 0010  0010    ----  ----            |  1 0  0010            |
> --| 0001  1---    ----  ----            |  1 0  1000            |
> --| 0001  01--    ----  ----            |  1 0  0100            |
> --| 0001  001-    ----  ----            |  1 0  0010            |
> --| 0001  0001    ----  ----            |  1 0  0001            |
> --| ----  0000    1000  -1--            |  0 1  0100            |
> --| ----  0000    1000  -01-            |  0 1  0010            |
> --| ----  0000    1000  -001            |  0 1  0001            |
> --| ----  0000    1000  1000            |  0 1  1000            |
> --| ----  0000    0100  --1-            |  0 1  0010            |
> --| ----  0000    0100  --01            |  0 1  0001            |
> --| ----  0000    0100  1-00            |  0 1  1000            |
> --| ----  0000    0100  0100            |  0 1  0100            |
> --| ----  0000    0010  ---1            |  0 1  0001            |
> --| ----  0000    0010  1--0            |  0 1  1000            |
> --| ----  0000    0010  01-0            |  0 1  0100            |
> --| ----  0000    0010  0010            |  0 1  0010            |
> --| ----  0000    0001  1---            |  0 1  1000            |
> --| ----  0000    0001  01--            |  0 1  0100            |
> --| ----  0000    0001  001-            |  0 1  0010            |
> --| ----  0000    0001  0001            |  0 1  0001            |
> --| ----  0000    ----  0000            |  0 0  ----            |
> --*END*=================================+=======================+
> --?TABLE END select_next;
Sky-HChina commented 3 years ago

Hi openpowerwtf, Following this question, what will happen if “ib_ic_empty" and "ib_ic_below_water" are both 0. From the last line of the table in your comment, it shows the tid selection is "----". What does this mean?

Many thanks

openpowerwtf commented 3 years ago

A '-' in the table in the output would mean don't-care. Both next-high-valid and next-low-valid are '0' for that case - I would expect they are qualifying all usages of next_tid in the surrounding logic.