From 72f8a321e9aa080c872742a6e2e5092b9fa5b37e Mon Sep 17 00:00:00 2001
From: Yifan Sun <sunyifan112358@gmail.com>
Date: Sat, 20 Apr 2019 00:10:48 -0400
Subject: [PATCH] Bug fix

---
 cache/writeback/bankstage.go           | 10 ++---
 cache/writeback/bankstage_test.go      |  4 --
 cache/writeback/bottomparser.go        | 21 ++++++++-
 cache/writeback/bottomparser_test.go   |  3 ++
 cache/writeback/directorystage.go      | 60 ++++++++++++++++++--------
 cache/writeback/directorystage_test.go | 46 +++++++++++++-------
 cache/writeback/mshrstage.go           |  4 +-
 cache/writeback/trace.go               | 30 +++++++------
 cache/writeback/transaction.go         | 16 ++++---
 cache/writeback/writebackcache.go      | 18 ++++----
 10 files changed, 138 insertions(+), 74 deletions(-)

diff --git a/cache/writeback/bankstage.go b/cache/writeback/bankstage.go
index b3d7a35..7d5fdc7 100644
--- a/cache/writeback/bankstage.go
+++ b/cache/writeback/bankstage.go
@@ -20,7 +20,6 @@ type bankStage struct {
 	mshrStageBuffer util.Buffer
 
 	inFlightTransactions *[]*transaction
-	pendingEvictions     *[]*transaction
 
 	cycleLeft    int
 	currentTrans *transaction
@@ -87,7 +86,7 @@ func (s *bankStage) finalizeReadHit(now akita.VTimeInSec) bool {
 	dataReady.Data = data
 	s.topSender.Send(dataReady)
 
-	trace("read done", addr, dataReady.Data)
+	trace(now, "read hit done", addr, dataReady.Data)
 
 	return true
 }
@@ -124,7 +123,7 @@ func (s *bankStage) finalizeWriteHit(now akita.VTimeInSec) bool {
 	done := mem.NewDoneRsp(now, s.topPort, write.Src(), write.ID)
 	s.topSender.Send(done)
 
-	trace("write done", addr, write.Data)
+	trace(now, "write hit done", addr, write.Data)
 
 	return true
 }
@@ -142,7 +141,7 @@ func (s *bankStage) finalizeBankWriteFetched(now akita.VTimeInSec) bool {
 	block.IsLocked = false
 	block.IsValid = true
 
-	trace("write fetched", block.Tag, mshrEntry.Data)
+	trace(now, "write fetched", block.Tag, mshrEntry.Data)
 
 	s.currentTrans = nil
 
@@ -178,10 +177,11 @@ func (s *bankStage) finalizeBankReadForEviction(now akita.VTimeInSec) bool {
 	write.Data = data
 	write.DirtyMask = victim.DirtyMask
 	trans.eviction = write
-	*s.pendingEvictions = append(*s.pendingEvictions, trans)
 	s.bottomSender.Send(write)
 
 	s.currentTrans = nil
 
+	trace(now, "evict", victim.Tag, data)
+
 	return true
 }
diff --git a/cache/writeback/bankstage_test.go b/cache/writeback/bankstage_test.go
index 2c1e2de..59a1388 100644
--- a/cache/writeback/bankstage_test.go
+++ b/cache/writeback/bankstage_test.go
@@ -18,7 +18,6 @@ var _ = Describe("Bank Stage", func() {
 		bottomSender         *MockBufferedSender
 		mshrStageBuffer      *MockBuffer
 		lowModuleFinder      *MockLowModuleFinder
-		pendingEvictions     []*transaction
 		inFlightTransactions []*transaction
 	)
 
@@ -30,7 +29,6 @@ var _ = Describe("Bank Stage", func() {
 		bottomSender = NewMockBufferedSender(mockCtrl)
 		lowModuleFinder = NewMockLowModuleFinder(mockCtrl)
 		storage = mem.NewStorage(4 * mem.KB)
-		pendingEvictions = nil
 		inFlightTransactions = nil
 
 		bs = &bankStage{
@@ -42,7 +40,6 @@ var _ = Describe("Bank Stage", func() {
 			bottomSender:         bottomSender,
 			lowModuleFinder:      lowModuleFinder,
 			mshrStageBuffer:      mshrStageBuffer,
-			pendingEvictions:     &pendingEvictions,
 			inFlightTransactions: &inFlightTransactions,
 		}
 	})
@@ -322,7 +319,6 @@ var _ = Describe("Bank Stage", func() {
 			ret := bs.Tick(10)
 			Expect(ret).To(BeTrue())
 			Expect(trans.eviction).To(BeIdenticalTo(evictionReq))
-			Expect(pendingEvictions).To(ContainElement(trans))
 			Expect(bs.currentTrans).To(BeNil())
 		})
 
diff --git a/cache/writeback/bottomparser.go b/cache/writeback/bottomparser.go
index 9e4b684..c092c6f 100644
--- a/cache/writeback/bottomparser.go
+++ b/cache/writeback/bottomparser.go
@@ -54,7 +54,7 @@ func (p *bottomParser) handleDataReady(
 		return false
 	}
 
-	trace("data ready", block.Tag, dr.Data)
+	trace(now, "data ready", block.Tag, dr.Data)
 
 	p.port.Retrieve(now)
 
@@ -110,6 +110,7 @@ func (p *bottomParser) handleDoneRsp(
 	}
 
 	trans := p.findEvictionTrans(done)
+	trans.evictionDone = done
 	if trans.mshrEntry != nil {
 		return p.fetch(now, trans)
 	}
@@ -137,6 +138,17 @@ func (p *bottomParser) findEvictionTrans(done *mem.DoneRsp) *transaction {
 	panic("pending eviction not found")
 }
 
+func (p *bottomParser) removeEvictionTrans(done *mem.DoneRsp) {
+	for i, t := range *p.pendingEvictions {
+		if t.eviction.ID == done.RespondTo {
+			*p.pendingEvictions = append((*p.pendingEvictions)[:i],
+				(*p.pendingEvictions)[i+1:]...)
+			return
+		}
+	}
+	panic("pending eviction not found")
+}
+
 func (p *bottomParser) fetch(now akita.VTimeInSec, trans *transaction) bool {
 	if !p.bottomSender.CanSend(1) {
 		return false
@@ -157,6 +169,10 @@ func (p *bottomParser) fetch(now akita.VTimeInSec, trans *transaction) bool {
 	p.bottomSender.Send(read)
 	p.port.Retrieve(now)
 
+	p.removeEvictionTrans(trans.evictionDone)
+
+	trace(now, "fetch (bottomParser)", read.Address, nil)
+
 	return true
 }
 
@@ -173,5 +189,8 @@ func (p *bottomParser) writeBank(
 	trans.bankAction = bankWriteHit
 	bankBuf.Push(trans)
 	p.port.Retrieve(now)
+
+	p.removeEvictionTrans(trans.evictionDone)
+
 	return true
 }
diff --git a/cache/writeback/bottomparser_test.go b/cache/writeback/bottomparser_test.go
index d3d320a..4a6f9a8 100644
--- a/cache/writeback/bottomparser_test.go
+++ b/cache/writeback/bottomparser_test.go
@@ -33,6 +33,7 @@ var _ = Describe("Bottom Parser", func() {
 		lowModuleFinder = NewMockLowModuleFinder(mockCtrl)
 		flusherBuffer = NewMockBuffer(mockCtrl)
 		state = cacheStateRunning
+		pendingEvictions = nil
 
 		parser = &bottomParser{
 			port:             port,
@@ -192,6 +193,7 @@ var _ = Describe("Bottom Parser", func() {
 
 			Expect(ret).To(BeTrue())
 			Expect(mshrEntry.ReadReq).To(BeIdenticalTo(readReq))
+			Expect(pendingEvictions).To(HaveLen(0))
 		})
 	})
 
@@ -247,6 +249,7 @@ var _ = Describe("Bottom Parser", func() {
 
 			Expect(ret).To(BeTrue())
 			Expect(evictionTrans.bankAction).To(Equal(bankWriteHit))
+			Expect(pendingEvictions).To(HaveLen(0))
 		})
 	})
 
diff --git a/cache/writeback/directorystage.go b/cache/writeback/directorystage.go
index 0c30f80..1d18c7e 100644
--- a/cache/writeback/directorystage.go
+++ b/cache/writeback/directorystage.go
@@ -8,14 +8,15 @@ import (
 )
 
 type directoryStage struct {
-	inBuf           util.Buffer
-	directory       cache.Directory
-	bankBufs        []util.Buffer
-	mshr            cache.MSHR
-	log2BlockSize   uint64
-	lowModuleFinder cache.LowModuleFinder
-	bottomSender    util.BufferedSender
-	toBottomPort    akita.Port
+	inBuf            util.Buffer
+	directory        cache.Directory
+	bankBufs         []util.Buffer
+	mshr             cache.MSHR
+	log2BlockSize    uint64
+	lowModuleFinder  cache.LowModuleFinder
+	bottomSender     util.BufferedSender
+	toBottomPort     akita.Port
+	pendingEvictions *[]*transaction
 }
 
 func (ds *directoryStage) Tick(now akita.VTimeInSec) bool {
@@ -35,7 +36,7 @@ func (ds *directoryStage) doRead(
 	now akita.VTimeInSec,
 	trans *transaction,
 ) bool {
-	trace("read start", trans.read.Address, nil)
+	trace(now, "read start", trans.read.Address, nil)
 	cachelineID, _ := GetCacheLineID(
 		trans.read.Address, ds.log2BlockSize)
 	block := ds.directory.Lookup(cachelineID)
@@ -79,7 +80,7 @@ func (ds *directoryStage) handleReadMiss(
 
 	victim := ds.directory.FindVictim(cacheLineID)
 	if ds.needEviction(victim) {
-		return ds.evict(trans, victim)
+		return ds.evict(now, trans, victim)
 	}
 
 	return ds.fetch(now, trans, victim)
@@ -90,7 +91,7 @@ func (ds *directoryStage) doWrite(
 	trans *transaction,
 ) bool {
 	write := trans.write
-	trace("write start", trans.write.Address, write.Data)
+	trace(now, "write start", trans.write.Address, write.Data)
 	cachelineID, _ := GetCacheLineID(write.Address, ds.log2BlockSize)
 	block := ds.directory.Lookup(cachelineID)
 
@@ -126,12 +127,12 @@ func (ds *directoryStage) doWriteMiss(
 	}
 
 	if ds.isWritingFullLine(write) {
-		return ds.writeFullLineMiss(trans)
+		return ds.writeFullLineMiss(now, trans)
 	}
 	return ds.writePartialLineMiss(now, trans)
 }
 
-func (ds *directoryStage) writeFullLineMiss(trans *transaction) bool {
+func (ds *directoryStage) writeFullLineMiss(now akita.VTimeInSec, trans *transaction) bool {
 	write := trans.write
 	cachelineID, _ := GetCacheLineID(write.Address, ds.log2BlockSize)
 
@@ -141,7 +142,7 @@ func (ds *directoryStage) writeFullLineMiss(trans *transaction) bool {
 	}
 
 	if ds.needEviction(victim) {
-		return ds.evict(trans, victim)
+		return ds.evict(now, trans, victim)
 	}
 
 	return ds.writeToBank(trans, victim)
@@ -164,7 +165,7 @@ func (ds *directoryStage) writePartialLineMiss(
 	}
 
 	if ds.needEviction(victim) {
-		return ds.evict(trans, victim)
+		return ds.evict(now, trans, victim)
 	}
 
 	return ds.fetch(now, trans, victim)
@@ -219,6 +220,7 @@ func (ds *directoryStage) writeToBank(
 }
 
 func (ds *directoryStage) evict(
+	now akita.VTimeInSec,
 	trans *transaction,
 	victim *cache.Block,
 ) bool {
@@ -261,6 +263,11 @@ func (ds *directoryStage) evict(
 	ds.inBuf.Pop()
 	bankBuf.Push(trans)
 
+	trans.evictingAddr = trans.victim.Tag
+	*ds.pendingEvictions = append(*ds.pendingEvictions, trans)
+
+	trace(now, "evict (dir)", trans.victim.Tag, nil)
+
 	return true
 }
 
@@ -269,9 +276,6 @@ func (ds *directoryStage) fetch(
 	trans *transaction,
 	block *cache.Block,
 ) bool {
-	if !ds.bottomSender.CanSend(1) {
-		return false
-	}
 
 	var addr uint64
 	if trans.read != nil {
@@ -279,10 +283,17 @@ func (ds *directoryStage) fetch(
 	} else {
 		addr = trans.write.Address
 	}
-
 	cacheLineID, _ := GetCacheLineID(
 		addr, uint64(ds.log2BlockSize))
 
+	if ds.isEvicting(cacheLineID) {
+		return false
+	}
+
+	if !ds.bottomSender.CanSend(1) {
+		return false
+	}
+
 	mshrEntry := ds.mshr.Add(cacheLineID)
 	trans.mshrEntry = mshrEntry
 	trans.block = block
@@ -302,6 +313,8 @@ func (ds *directoryStage) fetch(
 	)
 	ds.bottomSender.Send(read)
 
+	trace(now, "fetch (dir)", read.Address, nil)
+
 	mshrEntry.ReadReq = read
 	mshrEntry.Requests = append(mshrEntry.Requests, trans)
 	mshrEntry.Block = block
@@ -309,6 +322,15 @@ func (ds *directoryStage) fetch(
 	return true
 }
 
+func (ds *directoryStage) isEvicting(addr uint64) bool {
+	for _, t := range *ds.pendingEvictions {
+		if t.evictingAddr == addr {
+			return true
+		}
+	}
+	return false
+}
+
 func (ds *directoryStage) isWritingFullLine(write *mem.WriteReq) bool {
 	return len(write.Data) == (1 << ds.log2BlockSize)
 }
diff --git a/cache/writeback/directorystage_test.go b/cache/writeback/directorystage_test.go
index 667d8b5..b4a7dc1 100644
--- a/cache/writeback/directorystage_test.go
+++ b/cache/writeback/directorystage_test.go
@@ -13,14 +13,15 @@ import (
 var _ = Describe("DirectoryStage", func() {
 
 	var (
-		mockCtrl        *gomock.Controller
-		ds              *directoryStage
-		mshr            *MockMSHR
-		dirBuf          *MockBuffer
-		directory       *MockDirectory
-		bankBufs        []*MockBuffer
-		bottomSender    *MockBufferedSender
-		lowModuleFinder *MockLowModuleFinder
+		mockCtrl         *gomock.Controller
+		ds               *directoryStage
+		mshr             *MockMSHR
+		dirBuf           *MockBuffer
+		directory        *MockDirectory
+		bankBufs         []*MockBuffer
+		bottomSender     *MockBufferedSender
+		lowModuleFinder  *MockLowModuleFinder
+		pendingEvictions []*transaction
 	)
 
 	BeforeEach(func() {
@@ -33,15 +34,17 @@ var _ = Describe("DirectoryStage", func() {
 		bankBufs = append(bankBufs, NewMockBuffer(mockCtrl))
 		bankBufs = append(bankBufs, NewMockBuffer(mockCtrl))
 		lowModuleFinder = NewMockLowModuleFinder(mockCtrl)
+		pendingEvictions = nil
 
 		ds = &directoryStage{
-			log2BlockSize:   6,
-			inBuf:           dirBuf,
-			directory:       directory,
-			mshr:            mshr,
-			bankBufs:        []util.Buffer{bankBufs[0], bankBufs[1]},
-			bottomSender:    bottomSender,
-			lowModuleFinder: lowModuleFinder,
+			log2BlockSize:    6,
+			inBuf:            dirBuf,
+			directory:        directory,
+			mshr:             mshr,
+			bankBufs:         []util.Buffer{bankBufs[0], bankBufs[1]},
+			bottomSender:     bottomSender,
+			lowModuleFinder:  lowModuleFinder,
+			pendingEvictions: &pendingEvictions,
 		}
 	})
 
@@ -175,6 +178,17 @@ var _ = Describe("DirectoryStage", func() {
 				Expect(ret).To(BeFalse())
 			})
 
+			It("should not fetch if it is being evicting", func() {
+				trans := &transaction{
+					evictingAddr: 0x100,
+				}
+				pendingEvictions = append(pendingEvictions, trans)
+
+				ret := ds.Tick(10)
+
+				Expect(ret).To(BeFalse())
+			})
+
 			It("should create mshr entry and read from bottom", func() {
 				mshrEntry := &cache.MSHREntry{}
 				var readBottom *mem.ReadReq
@@ -272,6 +286,8 @@ var _ = Describe("DirectoryStage", func() {
 					true, true, true, true, false, false, false, false,
 					true, true, true, true, false, false, false, false,
 				}))
+				Expect(trans.evictingAddr).To(Equal(uint64(0x200)))
+				Expect(pendingEvictions).To(ContainElement(trans))
 				Expect(mshrEntry.Block).To(BeIdenticalTo(block))
 				Expect(mshrEntry.Requests).To(ContainElement(trans))
 			})
diff --git a/cache/writeback/mshrstage.go b/cache/writeback/mshrstage.go
index ec0c13d..5959691 100644
--- a/cache/writeback/mshrstage.go
+++ b/cache/writeback/mshrstage.go
@@ -62,7 +62,7 @@ func (s *mshrStage) respondRead(
 	dataReady.Data = data[offset : offset+read.MemByteSize]
 	s.topSender.Send(dataReady)
 
-	trace("read done", read.Address, dataReady.Data)
+	trace(now, "read done", read.Address, dataReady.Data)
 }
 
 func (s *mshrStage) respondWrite(
@@ -72,7 +72,7 @@ func (s *mshrStage) respondWrite(
 	done := mem.NewDoneRsp(now, s.topPort, write.Src(), write.ID)
 	s.topSender.Send(done)
 
-	trace("write done", write.Address, write.Data)
+	trace(now, "write done", write.Address, write.Data)
 }
 
 func (s *mshrStage) removeTransaction(trans *transaction) {
diff --git a/cache/writeback/trace.go b/cache/writeback/trace.go
index 0df6702..3f55580 100644
--- a/cache/writeback/trace.go
+++ b/cache/writeback/trace.go
@@ -1,15 +1,21 @@
 package writeback
 
-func trace(what string, addr uint64, data []byte) {
-	//s := ""
-	//s += fmt.Sprintf("%s, 0x%x", what, addr)
-	//if data != nil {
-	//	s += ", ["
-	//	for _, b := range data {
-	//		s += fmt.Sprintf("%02x, ", b)
-	//	}
-	//	s += "]"
-	//}
-	//s += "\n"
-	//fmt.Print(s)
+import (
+	"fmt"
+
+	"gitlab.com/akita/akita"
+)
+
+func trace(now akita.VTimeInSec, what string, addr uint64, data []byte) {
+	s := ""
+	s += fmt.Sprintf("%.15f, %s, 0x%x", now, what, addr)
+	if data != nil {
+		s += ", ["
+		for _, b := range data {
+			s += fmt.Sprintf("%02x, ", b)
+		}
+		s += "]"
+	}
+	s += "\n"
+	fmt.Print(s)
 }
diff --git a/cache/writeback/transaction.go b/cache/writeback/transaction.go
index e90217b..ad1cfaa 100644
--- a/cache/writeback/transaction.go
+++ b/cache/writeback/transaction.go
@@ -14,11 +14,13 @@ const (
 )
 
 type transaction struct {
-	read       *mem.ReadReq
-	write      *mem.WriteReq
-	block      *cache.Block
-	victim     *cache.Block
-	eviction   *mem.WriteReq
-	mshrEntry  *cache.MSHREntry
-	bankAction bankAction
+	read         *mem.ReadReq
+	write        *mem.WriteReq
+	block        *cache.Block
+	victim       *cache.Block
+	evictingAddr uint64
+	eviction     *mem.WriteReq
+	evictionDone *mem.DoneRsp
+	mshrEntry    *cache.MSHREntry
+	bankAction   bankAction
 }
diff --git a/cache/writeback/writebackcache.go b/cache/writeback/writebackcache.go
index 1aa4f4b..d6dfd0e 100644
--- a/cache/writeback/writebackcache.go
+++ b/cache/writeback/writebackcache.go
@@ -136,14 +136,15 @@ func NewWriteBackCache(
 	}
 
 	cache.dirStage = &directoryStage{
-		inBuf:           cache.dirStageBuffer,
-		directory:       directory,
-		mshr:            mshr,
-		bankBufs:        cache.bankBuffers,
-		log2BlockSize:   defaultLog2BlockSize,
-		lowModuleFinder: lowModuleFinder,
-		bottomSender:    cache.bottomSender,
-		toBottomPort:    cache.BottomPort,
+		inBuf:            cache.dirStageBuffer,
+		directory:        directory,
+		mshr:             mshr,
+		bankBufs:         cache.bankBuffers,
+		log2BlockSize:    defaultLog2BlockSize,
+		lowModuleFinder:  lowModuleFinder,
+		bottomSender:     cache.bottomSender,
+		toBottomPort:     cache.BottomPort,
+		pendingEvictions: &cache.pendingEvictions,
 	}
 
 	cache.bankStages = make([]*bankStage, 1)
@@ -158,7 +159,6 @@ func NewWriteBackCache(
 		bottomPort:           cache.BottomPort,
 		lowModuleFinder:      lowModuleFinder,
 		mshrStageBuffer:      cache.mshrStageBuffer,
-		pendingEvictions:     &cache.pendingEvictions,
 		inFlightTransactions: &cache.inFlightTransactions,
 	}
 
-- 
GitLab