From 8c47c0dd63f495a058e3a25906a39a352113c2a6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Tiago=20M=C3=BCck?= Date: Thu, 28 May 2020 21:36:40 -0500 Subject: [PATCH] cpu-o3: fix IQ missing mem barriers MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit After commit e2a5063e5f18f902833c84894b0ff103e3371493 some memory references now tracked as barriers were not having their completion properly notified to the MemDepUnit. This patch fixes InstructionQueue and changes MemDepUnit's completeBarrier to completeInst, which now should be called for both memory references and barrier instructions. Change-Id: I28b5f112b45778f6272e71bb3766b364c3d2e7db Signed-off-by: Tiago Mück Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/29654 Reviewed-by: Daniel Carvalho Maintainer: Jason Lowe-Power Tested-by: kokoro --- src/cpu/o3/inst_queue.hh | 3 --- src/cpu/o3/inst_queue_impl.hh | 36 +++++++++++---------------------- src/cpu/o3/mem_dep_unit.hh | 17 ++++++++-------- src/cpu/o3/mem_dep_unit_impl.hh | 2 +- 4 files changed, 22 insertions(+), 36 deletions(-) diff --git a/src/cpu/o3/inst_queue.hh b/src/cpu/o3/inst_queue.hh index f9e2966f0..b8878f3b1 100644 --- a/src/cpu/o3/inst_queue.hh +++ b/src/cpu/o3/inst_queue.hh @@ -240,9 +240,6 @@ class InstructionQueue /** Replays a memory instruction. It must be rescheduled first. */ void replayMemInst(const DynInstPtr &replay_inst); - /** Completes a memory operation. */ - void completeMemInst(const DynInstPtr &completed_inst); - /** * Defers a memory instruction when its DTB translation incurs a hw * page table walk. diff --git a/src/cpu/o3/inst_queue_impl.hh b/src/cpu/o3/inst_queue_impl.hh index 67b1108a3..ff5b3be4f 100644 --- a/src/cpu/o3/inst_queue_impl.hh +++ b/src/cpu/o3/inst_queue_impl.hh @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2014, 2017-2019 ARM Limited + * Copyright (c) 2011-2014, 2017-2020 ARM Limited * Copyright (c) 2013 Advanced Micro Devices, Inc. * All rights reserved. * @@ -1004,15 +1004,20 @@ InstructionQueue::wakeDependents(const DynInstPtr &completed_inst) // Tell the memory dependence unit to wake any dependents on this // instruction if it is a memory instruction. Also complete the memory // instruction at this point since we know it executed without issues. - // @todo: Might want to rename "completeMemInst" to something that - // indicates that it won't need to be replayed, and call this - // earlier. Might not be a big deal. + ThreadID tid = completed_inst->threadNumber; if (completed_inst->isMemRef()) { - memDepUnit[completed_inst->threadNumber].wakeDependents(completed_inst); - completeMemInst(completed_inst); + memDepUnit[tid].completeInst(completed_inst); + + DPRINTF(IQ, "Completing mem instruction PC: %s [sn:%llu]\n", + completed_inst->pcState(), completed_inst->seqNum); + + ++freeEntries; + completed_inst->memOpDone(true); + count[tid]--; } else if (completed_inst->isMemBarrier() || completed_inst->isWriteBarrier()) { - memDepUnit[completed_inst->threadNumber].completeBarrier(completed_inst); + // Completes a non mem ref barrier + memDepUnit[tid].completeInst(completed_inst); } for (int dest_reg_idx = 0; @@ -1121,23 +1126,6 @@ InstructionQueue::replayMemInst(const DynInstPtr &replay_inst) memDepUnit[replay_inst->threadNumber].replay(); } -template -void -InstructionQueue::completeMemInst(const DynInstPtr &completed_inst) -{ - ThreadID tid = completed_inst->threadNumber; - - DPRINTF(IQ, "Completing mem instruction PC: %s [sn:%llu]\n", - completed_inst->pcState(), completed_inst->seqNum); - - ++freeEntries; - - completed_inst->memOpDone(true); - - memDepUnit[tid].completed(completed_inst); - count[tid]--; -} - template void InstructionQueue::deferMemInst(const DynInstPtr &deferred_inst) diff --git a/src/cpu/o3/mem_dep_unit.hh b/src/cpu/o3/mem_dep_unit.hh index 3d24b1f13..685f6496b 100644 --- a/src/cpu/o3/mem_dep_unit.hh +++ b/src/cpu/o3/mem_dep_unit.hh @@ -139,14 +139,8 @@ class MemDepUnit */ void replay(); - /** Completes a memory instruction. */ - void completed(const DynInstPtr &inst); - - /** Completes a barrier instruction. */ - void completeBarrier(const DynInstPtr &inst); - - /** Wakes any dependents of a memory instruction. */ - void wakeDependents(const DynInstPtr &inst); + /** Notifies completion of an instruction. */ + void completeInst(const DynInstPtr &inst); /** Squashes all instructions up until a given sequence number for a * specific thread. @@ -164,6 +158,13 @@ class MemDepUnit void dumpLists(); private: + + /** Completes a memory instruction. */ + void completed(const DynInstPtr &inst); + + /** Wakes any dependents of a memory instruction. */ + void wakeDependents(const DynInstPtr &inst); + typedef typename std::list::iterator ListIt; class MemDepEntry; diff --git a/src/cpu/o3/mem_dep_unit_impl.hh b/src/cpu/o3/mem_dep_unit_impl.hh index d1eac2996..3a7ad363c 100644 --- a/src/cpu/o3/mem_dep_unit_impl.hh +++ b/src/cpu/o3/mem_dep_unit_impl.hh @@ -435,7 +435,7 @@ MemDepUnit::completed(const DynInstPtr &inst) template void -MemDepUnit::completeBarrier(const DynInstPtr &inst) +MemDepUnit::completeInst(const DynInstPtr &inst) { wakeDependents(inst); completed(inst); -- 2.30.2