From: Benjamin Herrenschmidt Date: Thu, 25 Aug 2022 03:14:20 +0000 (+1000) Subject: Fix PLRU X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=fa4baa2800178a9caaea18c4915ab39f04875c66;p=microwatt.git Fix PLRU Jacob Lifshay found a couple of issues with the PLRU implementation: - The tree array is one bit too long. This is harmless as this bit is never accessed and thus should be optimized out - The PLRU read is using the wrong nodes when going down the tree, which leads to incorrect results. This fixes it and improves the test bench a bit. I have verified the expected output using a hand-written tree states, observed the mismatch with the current implementation and verified the fix. Signed-off-by: Benjamin Herrenschmidt --- diff --git a/plru.vhdl b/plru.vhdl index 9d7d9c1..1c6533d 100644 --- a/plru.vhdl +++ b/plru.vhdl @@ -19,8 +19,12 @@ entity plru is end entity plru; architecture rtl of plru is + -- Each level of the tree (from leaf to root) has half the number of nodes + -- of the previous level. So for a 2^N bits LRU, we have a level of N/2 bits + -- one of N/4 bits etc.. down to 1. This gives us 2^N-1 nodes. Ie, 2 bits + -- LRU has 3 nodes (2 + 1), 4 bits LRU has 15 nodes (8 + 4 + 2 + 1) etc... constant count : positive := 2 ** BITS - 1; - subtype node_t is integer range 0 to count; + subtype node_t is integer range 0 to count - 1; type tree_t is array(node_t) of std_ulogic; signal tree: tree_t; @@ -31,14 +35,16 @@ begin -- in term of FPGA resource usage... get_lru: process(tree) variable node : node_t; + variable abit : std_ulogic; begin node := 0; for i in 0 to BITS-1 loop -- report "GET: i:" & integer'image(i) & " node:" & integer'image(node) & " val:" & std_ulogic'image(tree(node)); - lru(BITS-1-i) <= tree(node); + abit := tree(node); + lru(BITS-1-i) <= abit; if i /= BITS-1 then node := node * 2; - if tree(node) = '1' then + if abit = '1' then node := node + 2; else node := node + 1; @@ -69,7 +75,12 @@ begin end if; end if; end loop; - end if; + end if; end if; +-- if falling_edge(clk) then +-- if acc_en = '1' then +-- report "UPD: tree:" & to_string(tree); +-- end if; +-- end if; end process; end; diff --git a/plru_tb.vhdl b/plru_tb.vhdl index 63ca52d..2cd09f9 100644 --- a/plru_tb.vhdl +++ b/plru_tb.vhdl @@ -56,59 +56,75 @@ begin begin test_runner_setup(runner, runner_cfg); - wait for 4*clk_period; + wait for 8*clk_period; + + info("reset state:" & to_hstring(lru)); + check_equal(lru, 0, result("LRU")); info("accessing 1:"); acc <= "001"; acc_en <= '1'; wait for clk_period; info("lru:" & to_hstring(lru)); + check_equal(lru, 4, result("LRU")); info("accessing 2:"); acc <= "010"; wait for clk_period; info("lru:" & to_hstring(lru)); + check_equal(lru, 4, result("LRU")); info("accessing 7:"); acc <= "111"; wait for clk_period; info("lru:" & to_hstring(lru)); + check_equal(lru, 0, result("LRU")); info("accessing 4:"); acc <= "100"; wait for clk_period; info("lru:" & to_hstring(lru)); + check_equal(lru, 0, result("LRU")); info("accessing 3:"); acc <= "011"; wait for clk_period; info("lru:" & to_hstring(lru)); + check_equal(lru, 6, result("LRU")); info("accessing 5:"); acc <= "101"; wait for clk_period; info("lru:" & to_hstring(lru)); + check_equal(lru, 0, result("LRU")); info("accessing 3:"); acc <= "011"; wait for clk_period; info("lru:" & to_hstring(lru)); + check_equal(lru, 6, result("LRU")); info("accessing 5:"); acc <= "101"; wait for clk_period; info("lru:" & to_hstring(lru)); + check_equal(lru, 0, result("LRU")); info("accessing 6:"); acc <= "110"; wait for clk_period; info("lru:" & to_hstring(lru)); + check_equal(lru, 0, result("LRU")); info("accessing 0:"); acc <= "000"; wait for clk_period; info("lru:" & to_hstring(lru)); + check_equal(lru, 4, result("LRU")); + wait for clk_period; + wait for clk_period; + test_runner_cleanup(runner); end process; end;