From 85b4a738a92631a0bf9573f5a6748886e0b4cb2e Mon Sep 17 00:00:00 2001 From: Jessica Clarke Date: Wed, 8 Apr 2020 04:21:13 +0100 Subject: [PATCH 1/3] FetchStage.bsv: Fix a mkRegU that should be a mkReg We should not use a mkRegU for rg_pending_n_items, as otherwise there is a chance the epochs for rg_pending_f32d happen to match depending on the uninitialised values. Instead ensure we always reset with no pending items. --- src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv b/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv index f848372..bd8a66b 100644 --- a/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv +++ b/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv @@ -390,7 +390,7 @@ module mkFetchStage(FetchStage); Ehr #(2, Maybe #(Tuple3 #(Addr, Bit #(16), Bool))) ehr_pending_straddle <- mkEhr(tagged Invalid); // Reg to hold extra instructions from Fetch3 to send to decode the next cycle Reg #(Vector #(SupSizeX2S1, Inst_Item)) rg_pending_decode <- mkReg(replicate(defaultValue)); - Reg #(SupCntX2S1) rg_pending_n_items <- mkRegU; + Reg #(SupCntX2S1) rg_pending_n_items <- mkReg(0); Reg #(Fetch3ToDecode) rg_pending_f32d <- mkRegU; // Pipeline Stage FIFOs From 10c983d101a0602c706ea00a9424879ee76beb9c Mon Sep 17 00:00:00 2001 From: Jessica Clarke Date: Wed, 8 Apr 2020 04:22:49 +0100 Subject: [PATCH 2/3] FetchStage.bsv: Drop unused pc from Fetch3ToDecode Each instruction carries its own PC by this point, so it just duplicates the PC of the first item in the vector. Moreover, when including a pending straddle at the head, the PC value was not set to the previous half's, ie PC-2. We keep pred_next_pc as that's used to determine where the bundle is predicted to go next, and the duplication avoids having to dynamically index the vector based on the number of instructions fed to decode. --- src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv | 3 --- 1 file changed, 3 deletions(-) diff --git a/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv b/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv index bd8a66b..a10af69 100644 --- a/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv +++ b/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv @@ -125,7 +125,6 @@ typedef struct { } Fetch2ToFetch3 deriving(Bits, Eq, FShow); typedef struct { - Addr pc; Addr pred_next_pc; Bool mispred_first_half; Maybe#(Exception) cause; @@ -692,7 +691,6 @@ module mkFetchStage(FetchStage); if (pending_n_items == 0) begin out = Fetch3ToDecode { - pc: fetch3In.pc, pred_next_pc: pred_next_pc, mispred_first_half: mispred_first_half, cause: fetch3In.cause, @@ -742,7 +740,6 @@ module mkFetchStage(FetchStage); next_pending_n_items = truncate(n_items - fromInteger(valueOf(SupSize))); rg_pending_decode <= drop(v_items); rg_pending_f32d <= Fetch3ToDecode { - pc: v_items[valueOf(SupSize)].pc, pred_next_pc: out.pred_next_pc, mispred_first_half: False, cause: tagged Invalid, From 8455ee3f0644e0228ef8c1131fcf93c0cce84491 Mon Sep 17 00:00:00 2001 From: Jessica Clarke Date: Wed, 8 Apr 2020 04:47:55 +0100 Subject: [PATCH 3/3] FetchStage.bsv: Fix fav_parse_insts when pending_straddle has "even" PC Despite the name, we can end up having a pending_straddle at an "even" PC (and thus an "odd" pc_start). In this case, we would erroneously treat it as if pc_start were "even", effectively shifting all the instruction parcels along by one and duplicating the first one, since the first element of the parcels vector will be a copy of the straddle's parcel itself. Instead, we should just ignore pending_straddle when determining where to start in the parcels vector, as the two are completely independent, and no longer hard-code that the straddle is merged with element 0. This can happen if we predict that the first half of an uncompressed instruction is a taken branch. We will have the instruction bytes available, but not the prediction information, so still have to do a normal Fetch3 redirect in the same manner as an unaligned uncompressed instruction. We could perhaps special-case with a fast-path (or query the predictor directly in Fetch3), but it should hit in the TLB and L1 cache, and is probably not common enough to warrant the complexity. The FreeRTOS-based netboot loader trips up on this as it overwrites itself with the payload, and thus all the branch prediction still has many taken branch entries, most of which will not be correct. --- src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv b/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv index a10af69..9cd18ee 100644 --- a/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv +++ b/src_Core/RISCY_OOO/procs/RV64G_OOO/FetchStage.bsv @@ -272,8 +272,8 @@ function ActionValue #(Tuple4 #(SupCntX2, orig_inst: 0, inst: 0}); Maybe #(Tuple3 #(Addr, Bit #(16), Bool)) next_straddle = tagged Invalid; - // Start parse at parcel 0/1 depending on pc lsbs and pending straddle - SupCntX2 j = ((pc_start [1:0] == 2'b00 || isValid(pending_straddle)) ? 0 : 1); + // Start parse at parcel 0/1 depending on pc lsbs. + SupCntX2 j = (pc_start [1:0] == 2'b00 ? 0 : 1); Addr pc = pc_start; Integer n_items = 0; for (Integer i = 0; i < valueOf (SupSizeX2); i = i + 1) begin @@ -289,9 +289,9 @@ function ActionValue #(Tuple4 #(SupCntX2, end pc = s_pc; inst_kind = Inst_32b; - orig_inst = { v_x16[0], s_lsbs }; + orig_inst = { v_x16[j], s_lsbs }; inst = orig_inst; - j = 1; + j = j + 1; next_pc = s_pc + 4; n_items = 1; end