[prim_arb_tree/rv_plic_target] Remove TODOs due to a Vivado tool bug
This tool bug has been fixed in Vivado 2020.2.
Fix #1408
Signed-off-by: Michael Schaffner <msf@google.com>
diff --git a/hw/ip/prim/rtl/prim_arbiter_tree.sv b/hw/ip/prim/rtl/prim_arbiter_tree.sv
index 16281fb..7b6be73 100644
--- a/hw/ip/prim/rtl/prim_arbiter_tree.sv
+++ b/hw/ip/prim/rtl/prim_arbiter_tree.sv
@@ -141,33 +141,25 @@
// local helper variable
logic sel;
- // TODO: The always_comb code is split into two blocks to allow Verilator to schedule them
- // separately (avoiding a spurious UNOPTFLAT warning). The whole lot would probably
- // be clearer as a set of continuous assignments, rather than using always_comb
- // blocks. Unfortunately, we can't currently do that because of a Vivado bug,
- // reported in January 2020. This is tracked with OpenTitan issue #1408. There's
- // currently no information about a Vivado version with this fixed.
- always_comb begin : p_sel
- // forward path (requests and data)
- // each node looks at its two children, and selects the one with higher priority
- sel = ~req_tree[C0] | ~prio_tree[C0] & prio_tree[C1];
- end
- always_comb begin : p_node
- // propagate requests
- req_tree[Pa] = req_tree[C0] | req_tree[C1];
- prio_tree[Pa] = prio_tree[C1] | prio_tree[C0];
- // data and index muxes
- idx_tree[Pa] = (sel) ? idx_tree[C1] : idx_tree[C0];
- data_tree[Pa] = (sel) ? data_tree[C1] : data_tree[C0];
+ // forward path (requests and data)
+ // each node looks at its two children, and selects the one with higher priority
+ assign sel = ~req_tree[C0] | ~prio_tree[C0] & prio_tree[C1];
+ // propagate requests
+ assign req_tree[Pa] = req_tree[C0] | req_tree[C1];
+ assign prio_tree[Pa] = prio_tree[C1] | prio_tree[C0];
+ // data and index muxes
+ // Note: these ternaries have triggered a synthesis bug in Vivado versions older
+ // than 2020.2. If the problem resurfaces again, have a look at issue #1408.
+ assign idx_tree[Pa] = (sel) ? idx_tree[C1] : idx_tree[C0];
+ assign data_tree[Pa] = (sel) ? data_tree[C1] : data_tree[C0];
- // backward path (grants and prefix sum)
- // this propagates the selction index back and computes a hot one mask
- sel_tree[C0] = sel_tree[Pa] & ~sel;
- sel_tree[C1] = sel_tree[Pa] & sel;
- // this performs a prefix sum for masking the input requests in the next cycle
- mask_tree[C0] = mask_tree[Pa];
- mask_tree[C1] = mask_tree[Pa] | sel_tree[C0];
- end
+ // backward path (grants and prefix sum)
+ // this propagates the selction index back and computes a hot one mask
+ assign sel_tree[C0] = sel_tree[Pa] & ~sel;
+ assign sel_tree[C1] = sel_tree[Pa] & sel;
+ // this performs a prefix sum for masking the input requests in the next cycle
+ assign mask_tree[C0] = mask_tree[Pa];
+ assign mask_tree[C1] = mask_tree[Pa] | sel_tree[C0];
end
end : gen_level
end : gen_tree
diff --git a/hw/ip/rv_plic/rtl/rv_plic_target.sv b/hw/ip/rv_plic/rtl/rv_plic_target.sv
index 2447e78..ded960e 100644
--- a/hw/ip/rv_plic/rtl/rv_plic_target.sv
+++ b/hw/ip/rv_plic/rtl/rv_plic_target.sv
@@ -79,32 +79,17 @@
end
// this creates the node assignments
end else begin : gen_nodes
- // NOTE: the code below has been written in this way in order to work
- // around a synthesis issue in Vivado 2018.3 and 2019.2 where the whole
- // module would be optimized away if these assign statements contained
- // ternary statements to implement the muxes.
- //
- // TODO: rewrite these lines with ternary statmements onec the problem
- // has been fixed in the tool.
- //
- // See also originating issue:
- // https://github.com/lowRISC/opentitan/issues/1355
- // Xilinx issue:
- // https://forums.xilinx.com/t5/Synthesis/
- // Simulation-Synthesis-Mismatch-with-Vivado-2018-3/m-p/1065923#M33849
-
logic sel; // local helper variable
// in case only one of the parent has a pending irq_o, forward that one
// in case both irqs are pending, forward the one with higher priority
assign sel = (~is_tree[C0] & is_tree[C1]) |
(is_tree[C0] & is_tree[C1] & logic'(max_tree[C1] > max_tree[C0]));
// forwarding muxes
- assign is_tree[Pa] = (sel & is_tree[C1]) |
- ((~sel) & is_tree[C0]);
- assign id_tree[Pa] = ({SrcWidth{sel}} & id_tree[C1]) |
- ({SrcWidth{~sel}} & id_tree[C0]);
- assign max_tree[Pa] = ({PrioWidth{sel}} & max_tree[C1]) |
- ({PrioWidth{~sel}} & max_tree[C0]);
+ // Note: these ternaries have triggered a synthesis bug in Vivado versions older
+ // than 2020.2. If the problem resurfaces again, have a look at issue #1408.
+ assign is_tree[Pa] = (sel) ? is_tree[C1] : is_tree[C0];
+ assign id_tree[Pa] = (sel) ? id_tree[C1] : id_tree[C0];
+ assign max_tree[Pa] = (sel) ? max_tree[C1] : max_tree[C0];
end
end : gen_level
end : gen_tree
