Skip to content

Commit 6388b8d

Browse files
committed
misc: remove redundant test & counter
Change-Id: I6f1de90ea368558913dc913593ac0f193f54976b
1 parent 1c9ec5a commit 6388b8d

File tree

7 files changed

+22
-242
lines changed

7 files changed

+22
-242
lines changed

docs/Gem5_Docs/frontend/full_resolve_train_review_guide.md

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ These files now:
127127
- track FTQ target generation identity
128128
- validate full resolve packets using structural checks
129129
- dispatch full resolve packets to migrated components
130+
- expose only the retained top-level resolve-train counters:
131+
`fullResolveTrainAccepted`, `fullResolveTrainRejectTargetMismatch`,
132+
`fullResolveTrainRejectPacketValidation`, and
133+
`fullResolveTrainRejectComponent`
130134
- preserve commit/update path for non-resolved-stage training
131135

132136
### Migrated predictor components
@@ -209,11 +213,23 @@ migrated BTB predictors, not through a legacy helper chain.
209213

210214
### Unit tests
211215

212-
- `build/RISCV/cpu/pred/btb/test/tage.test.debug`
216+
- build: `scons build/RISCV/cpu/pred/btb/test/tage.test.debug --unit-test -j60`
217+
- run: `build/RISCV/cpu/pred/btb/test/tage.test.debug --gtest_filter=BTBTAGETest.*`
213218

214-
Current result:
219+
Current retained coverage:
215220

216-
- `22/22` passing
221+
- the existing `BTBTAGETest.*` suite remains in place
222+
- within the branch-added cleanup surface, the retained regressions are:
223+
`NewConditionalEntryWithoutPredictionMetaStillTrains`,
224+
`ResolveTrainBankConflict`,
225+
`ResolveTrainUsesPacketTruthForConditionalSelection`, and
226+
`ResolveTrainRepeatedShortPatternMatchesLegacyProviderGrowth`
227+
228+
Removed from the branch verification surface:
229+
230+
- rollout-time debug counters
231+
- legacy resolve-update-only `BankConflict`
232+
- exploratory `BTBTAGEUpperBound*` checks added during branch development
217233

218234
### Targeted workloads
219235

src/cpu/o3/fetch.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,6 @@ Fetch::FetchStatGroup::FetchStatGroup(CPU *cpu, Fetch *fetch)
266266
frontendBound - frontendLatencyBound),
267267
ADD_STAT(fullResolveEntriesReceived, statistics::units::Count::get(),
268268
"Number of full resolve entries received by fetch"),
269-
ADD_STAT(fullResolveEntriesMerged, statistics::units::Count::get(),
270-
"Number of full resolve entries merged by fetch"),
271269
ADD_STAT(fullResolveEntriesDroppedQueueFull,
272270
statistics::units::Count::get(),
273271
"Number of full resolve entries dropped because the queue is full"),
@@ -358,8 +356,6 @@ Fetch::FetchStatGroup::FetchStatGroup(CPU *cpu, Fetch *fetch)
358356
.flags(statistics::total);
359357
fullResolveEntriesReceived
360358
.prereq(fullResolveEntriesReceived);
361-
fullResolveEntriesMerged
362-
.prereq(fullResolveEntriesMerged);
363359
fullResolveEntriesDroppedQueueFull
364360
.prereq(fullResolveEntriesDroppedQueueFull);
365361
fullResolveEntriesDroppedStaleTarget
@@ -1520,7 +1516,6 @@ Fetch::handleIEWSignals()
15201516
resolved.mispredict, resolved.ftqOffset, resolved.isCond,
15211517
resolved.isDirect, resolved.isIndirect, resolved.isCall,
15221518
resolved.isReturn, resolved.isRVC));
1523-
fetchStats.fullResolveEntriesMerged++;
15241519
continue;
15251520
}
15261521

src/cpu/o3/fetch.hh

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,8 +1125,6 @@ class Fetch
11251125
statistics::Formula frontendBandwidthBound;
11261126
/** Full resolve entries observed at fetch. */
11271127
statistics::Scalar fullResolveEntriesReceived;
1128-
/** Full resolve entries merged with an existing target. */
1129-
statistics::Scalar fullResolveEntriesMerged;
11301128
/** Full resolve entries dropped because the queue is full. */
11311129
statistics::Scalar fullResolveEntriesDroppedQueueFull;
11321130
/** Full resolve entries dropped because the target went stale. */

src/cpu/pred/btb/decoupled_bpred.cc

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,27 +30,22 @@ DecoupledBPUWithBTB::validateResolvedTrainPacket(const ResolvedTrainPacket &pack
3030
bool seenTaken = false;
3131
for (const auto &resolved : packet.realBranches) {
3232
if (resolved.branch.pc < packet.startPC) {
33-
dbpBtbStats.fullResolveTrainValidationPcBeforeStart++;
3433
return ResolveTrainValidationReason::PcBeforeStart;
3534
}
3635

3736
if (resolved.branch.size == 0) {
38-
dbpBtbStats.fullResolveTrainValidationZeroSize++;
3937
return ResolveTrainValidationReason::ZeroSize;
4038
}
4139

4240
if (seenTaken) {
43-
dbpBtbStats.fullResolveTrainValidationAfterTaken++;
4441
return ResolveTrainValidationReason::AfterTaken;
4542
}
4643

4744
if (!firstBranch) {
4845
if (resolved.ftqOffset < lastOffset) {
49-
dbpBtbStats.fullResolveTrainValidationOffsetReversed++;
5046
return ResolveTrainValidationReason::OffsetReversed;
5147
}
5248
if (resolved.ftqOffset == lastOffset && resolved.branch.pc <= lastPc) {
53-
dbpBtbStats.fullResolveTrainValidationPcOrderSameOffset++;
5449
return ResolveTrainValidationReason::PcOrderSameOffset;
5550
}
5651
}
@@ -638,7 +633,6 @@ DecoupledBPUWithBTB::resolveTrain(
638633
DPRINTF(DecoupleBP,
639634
"Resolve-train packet tid mismatch: packet=%u arg=%u\n",
640635
packet.tid, tid);
641-
dbpBtbStats.fullResolveTrainRejectTidMismatch++;
642636
return false;
643637
}
644638

@@ -656,7 +650,6 @@ DecoupledBPUWithBTB::resolveTrain(
656650
DPRINTF(DecoupleBP,
657651
"Resolve-train packet startPC mismatch: packet=%#lx ftq=%#lx id=%lu tid=%u\n",
658652
packet.startPC, target.startPC, packet.target.id, tid);
659-
dbpBtbStats.fullResolveTrainRejectStartPCMismatch++;
660653
return false;
661654
}
662655

src/cpu/pred/btb/decoupled_bpred.hh

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -314,19 +314,10 @@ class DecoupledBPUWithBTB : public BPredUnit
314314

315315
// Full resolve train reject statistics
316316
statistics::Scalar fullResolveTrainAccepted;
317-
statistics::Scalar fullResolveTrainRejectTidMismatch;
318317
statistics::Scalar fullResolveTrainRejectTargetMismatch;
319-
statistics::Scalar fullResolveTrainRejectStartPCMismatch;
320318
statistics::Scalar fullResolveTrainRejectPacketValidation;
321319
statistics::Scalar fullResolveTrainRejectComponent;
322320

323-
// Full resolve train validation sub-reasons
324-
statistics::Scalar fullResolveTrainValidationPcBeforeStart;
325-
statistics::Scalar fullResolveTrainValidationZeroSize;
326-
statistics::Scalar fullResolveTrainValidationAfterTaken;
327-
statistics::Scalar fullResolveTrainValidationOffsetReversed;
328-
statistics::Scalar fullResolveTrainValidationPcOrderSameOffset;
329-
330321
statistics::Scalar s1PredWrongFallthrough;
331322
statistics::Scalar s1PredWrongUbtb;
332323
statistics::Scalar s1PredWrongAbtb;

src/cpu/pred/btb/decoupled_bpred_stats.cc

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -460,26 +460,12 @@ DecoupledBPUWithBTB::DBPBTBStats::DBPBTBStats(
460460
ADD_STAT(predictionBlockedForUpdate, statistics::units::Count::get(), "prediction blocked for update priority"),
461461
ADD_STAT(fullResolveTrainAccepted, statistics::units::Count::get(),
462462
"accepted full resolve-train packets"),
463-
ADD_STAT(fullResolveTrainRejectTidMismatch, statistics::units::Count::get(),
464-
"full resolve-train packets rejected due to thread id mismatch"),
465463
ADD_STAT(fullResolveTrainRejectTargetMismatch, statistics::units::Count::get(),
466464
"full resolve-train packets rejected due to FTQ target mismatch"),
467-
ADD_STAT(fullResolveTrainRejectStartPCMismatch, statistics::units::Count::get(),
468-
"full resolve-train packets rejected due to startPC mismatch"),
469465
ADD_STAT(fullResolveTrainRejectPacketValidation, statistics::units::Count::get(),
470466
"full resolve-train packets rejected due to packet validation failure"),
471467
ADD_STAT(fullResolveTrainRejectComponent, statistics::units::Count::get(),
472468
"full resolve-train packets rejected by predictor components"),
473-
ADD_STAT(fullResolveTrainValidationPcBeforeStart, statistics::units::Count::get(),
474-
"packet validation failures because branch pc is before startPC"),
475-
ADD_STAT(fullResolveTrainValidationZeroSize, statistics::units::Count::get(),
476-
"packet validation failures because branch size is zero"),
477-
ADD_STAT(fullResolveTrainValidationAfterTaken, statistics::units::Count::get(),
478-
"packet validation failures because another branch appears after a taken branch"),
479-
ADD_STAT(fullResolveTrainValidationOffsetReversed, statistics::units::Count::get(),
480-
"packet validation failures because ftq offsets are not monotonic"),
481-
ADD_STAT(fullResolveTrainValidationPcOrderSameOffset, statistics::units::Count::get(),
482-
"packet validation failures because pcs at same offset are not strictly increasing"),
483469
ADD_STAT(s1PredWrongFallthrough, statistics::units::Count::get(), "S1pred wrong full throughs"),
484470
ADD_STAT(s1PredWrongUbtb, statistics::units::Count::get(),"S1pred wrong using ubtb "),
485471
ADD_STAT(s1PredWrongAbtb, statistics::units::Count::get(), "S1pred wrong using abtb "),

src/cpu/pred/btb/test/btb_tage.test.cc

Lines changed: 3 additions & 202 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
#include "base/types.hh"
77
#include "cpu/pred/btb/btb_tage.hh"
8-
#include "cpu/pred/btb/btb_tage_ub.hh"
98
#include "cpu/pred/btb/common.hh"
109
#include "cpu/pred/btb/folded_hist.hh"
1110

@@ -1064,83 +1063,11 @@ TEST_F(BTBTAGETest, NewConditionalEntryWithoutPredictionMetaStillTrains) {
10641063
}
10651064

10661065
/**
1067-
* @brief Test bank conflict detection
1066+
* @brief Test resolve-train bank conflict detection
10681067
*
1069-
* Verifies:
1070-
* 1. Same bank access causes conflict and drops update (when enabled)
1071-
* 2. Different bank access has no conflict
1072-
* 3. Disabled flag prevents conflict detection
1068+
* Verifies that a same-bank resolve-train update is rejected when bank
1069+
* conflict checking is enabled.
10731070
*/
1074-
TEST_F(BTBTAGETest, BankConflict) {
1075-
// Create TAGE with 4 banks
1076-
BTBTAGE *bankTage = new BTBTAGE(4, 2, 1024, 4);
1077-
boost::dynamic_bitset<> testHistory(128);
1078-
std::vector<FullBTBPrediction> testStagePreds(5);
1079-
1080-
// Bank ID derives from bits [2:1] (pc >> 1) & 0x3 when instShiftAmt == 1.
1081-
// Bank 0: ..., 0x100, 0x108 ... Bank 1: ..., 0x102, 0x10A ...
1082-
// Bank 2: ..., 0x104, 0x10C ... Bank 3: ..., 0x106, 0x10E ...
1083-
1084-
// Test 1: Same bank conflict (enabled)
1085-
bankTage->enableBankConflict = true;
1086-
{
1087-
// Predict on bank 1 (0x20), then update on bank 1 (0xa0)
1088-
testStagePreds[1].btbEntries = {createBTBEntry(0x20)};
1089-
bankTage->putPCHistory(0x20, testHistory, testStagePreds);
1090-
EXPECT_TRUE(bankTage->predBankValid);
1091-
1092-
auto meta = bankTage->getPredictionMeta();
1093-
FetchTarget stream = createStream(0xa0, createBTBEntry(0xa0), true, meta);
1094-
setupTageEntry(bankTage, 0xa0, 0, 1, false);
1095-
1096-
uint64_t conflicts_before = bankTage->tageStats.updateBankConflict;
1097-
bool can_update = bankTage->canResolveUpdate(stream);
1098-
1099-
// Should detect conflict and defer update
1100-
EXPECT_EQ(bankTage->tageStats.updateBankConflict, conflicts_before + 1);
1101-
EXPECT_FALSE(can_update);
1102-
EXPECT_FALSE(bankTage->predBankValid);
1103-
}
1104-
1105-
// Test 2: Different bank, no conflict
1106-
{
1107-
// Predict on bank 0 (0x100), update on bank 2 (0x104)
1108-
testStagePreds[1].btbEntries = {createBTBEntry(0x100)};
1109-
bankTage->putPCHistory(0x100, testHistory, testStagePreds);
1110-
1111-
auto meta = bankTage->getPredictionMeta();
1112-
FetchTarget stream = createStream(0x104, createBTBEntry(0x104), true, meta);
1113-
1114-
uint64_t conflicts_before = bankTage->tageStats.updateBankConflict;
1115-
bool can_update = bankTage->canResolveUpdate(stream);
1116-
ASSERT_TRUE(can_update);
1117-
bankTage->doResolveUpdate(stream);
1118-
1119-
// Should not detect conflict
1120-
EXPECT_EQ(bankTage->tageStats.updateBankConflict, conflicts_before);
1121-
}
1122-
1123-
// Test 3: Disabled flag prevents conflict
1124-
bankTage->enableBankConflict = false;
1125-
{
1126-
// Same bank (0x20 and 0xa0), but conflict disabled
1127-
testStagePreds[1].btbEntries = {createBTBEntry(0x20)};
1128-
bankTage->putPCHistory(0x20, testHistory, testStagePreds);
1129-
1130-
auto meta = bankTage->getPredictionMeta();
1131-
FetchTarget stream = createStream(0xa0, createBTBEntry(0xa0), true, meta);
1132-
setupTageEntry(bankTage, 0xa0, 0, 1, false);
1133-
1134-
uint64_t conflicts_before = bankTage->tageStats.updateBankConflict;
1135-
bool can_update = bankTage->canResolveUpdate(stream);
1136-
ASSERT_TRUE(can_update);
1137-
bankTage->doResolveUpdate(stream);
1138-
1139-
// No conflict even with same bank
1140-
EXPECT_EQ(bankTage->tageStats.updateBankConflict, conflicts_before);
1141-
}
1142-
}
1143-
11441071
TEST_F(BTBTAGETest, ResolveTrainBankConflict) {
11451072
BTBTAGE bankTage(4, 2, 1024, 4);
11461073
memset(&bankTage.tageStats, 0, sizeof(BTBTAGE::TageStats));
@@ -1282,132 +1209,6 @@ TEST_F(BTBTAGETest, ResolveTrainRepeatedShortPatternMatchesLegacyProviderGrowth)
12821209
<< "Full resolve-train should build the same set of TAGE tables as legacy update";
12831210
}
12841211

1285-
class BTBTAGEUpperBoundTest : public ::testing::Test
1286-
{
1287-
protected:
1288-
void SetUp() override {
1289-
tage = new BTBTAGEUpperBound();
1290-
memset(&tage->tageStats, 0, sizeof(BTBTAGE::TageStats));
1291-
history.resize(128, false);
1292-
stagePreds.resize(2);
1293-
}
1294-
1295-
BTBTAGEUpperBound *tage;
1296-
boost::dynamic_bitset<> history;
1297-
std::vector<FullBTBPrediction> stagePreds;
1298-
};
1299-
1300-
class BTBTAGEUpperBoundPathHashTest : public ::testing::Test
1301-
{
1302-
protected:
1303-
void SetUp() override {
1304-
tage = new BTBTAGEUpperBound(4, 1024, 4,
1305-
BTBTAGEUpperBound::HistorySource::PathHash);
1306-
memset(&tage->tageStats, 0, sizeof(BTBTAGE::TageStats));
1307-
outcomeHistory.resize(128, false);
1308-
pathHistory.resize(128, false);
1309-
stagePreds.resize(2);
1310-
}
1311-
1312-
BTBTAGEUpperBound *tage;
1313-
boost::dynamic_bitset<> outcomeHistory;
1314-
boost::dynamic_bitset<> pathHistory;
1315-
std::vector<FullBTBPrediction> stagePreds;
1316-
};
1317-
1318-
TEST_F(BTBTAGEUpperBoundTest, ExactContextLookup) {
1319-
BTBEntry entry = createBTBEntry(0x1000, true, true, false, -1);
1320-
boost::dynamic_bitset<> historyA(128, 0);
1321-
boost::dynamic_bitset<> historyB(128, 0);
1322-
historyB[0] = true;
1323-
1324-
ASSERT_TRUE(tage->insertExactEntry(3, entry.pc, historyA, 2));
1325-
EXPECT_TRUE(tage->hasExactEntry(3, entry.pc, historyA));
1326-
EXPECT_FALSE(tage->hasExactEntry(3, entry.pc, historyB));
1327-
1328-
bool predA = predictTAGE(tage, 0x1000, {entry}, historyA, stagePreds);
1329-
bool predB = predictTAGE(tage, 0x1000, {entry}, historyB, stagePreds);
1330-
1331-
EXPECT_TRUE(predA);
1332-
EXPECT_FALSE(predB);
1333-
}
1334-
1335-
TEST_F(BTBTAGEUpperBoundTest, ProviderAltSelection) {
1336-
BTBEntry entry = createBTBEntry(0x1000, true, true, false, -1);
1337-
1338-
ASSERT_TRUE(tage->insertExactEntry(3, entry.pc, history, 0));
1339-
ASSERT_TRUE(tage->insertExactEntry(1, entry.pc, history, -2));
1340-
1341-
predictTAGE(tage, 0x1000, {entry}, history, stagePreds);
1342-
auto meta = std::static_pointer_cast<BTBTAGE::TageMeta>(tage->getPredictionMeta());
1343-
auto pred = meta->preds[entry.pc];
1344-
1345-
EXPECT_EQ(pred.mainInfo.table, 3u);
1346-
EXPECT_EQ(pred.altInfo.table, 1u);
1347-
EXPECT_TRUE(pred.useAlt);
1348-
EXPECT_FALSE(pred.taken);
1349-
}
1350-
1351-
TEST_F(BTBTAGEUpperBoundTest, AllocationUsesPredictionTimeHistory) {
1352-
BTBEntry entry = createBTBEntry(0x1000, true, true, false, -1);
1353-
boost::dynamic_bitset<> historyA(128, 0);
1354-
boost::dynamic_bitset<> historyB(128, 0);
1355-
historyB[0] = true;
1356-
1357-
predictTAGE(tage, 0x1000, {entry}, historyA, stagePreds);
1358-
auto meta = tage->getPredictionMeta();
1359-
1360-
FetchTarget stream = createStream(0x1000, entry, true, meta);
1361-
stream = setMispredStream(stream);
1362-
1363-
tage->recoverHist(historyB, stream, 1, true);
1364-
tage->update(stream);
1365-
1366-
EXPECT_TRUE(tage->hasExactEntry(0, entry.pc, historyA));
1367-
EXPECT_FALSE(tage->hasExactEntry(0, entry.pc, historyB));
1368-
}
1369-
1370-
TEST_F(BTBTAGEUpperBoundTest, NewConditionalEntryWithoutPredictionMetaStillTrains) {
1371-
boost::dynamic_bitset<> historyA(128, 0);
1372-
stagePreds[1].btbEntries.clear();
1373-
tage->putPCHistory(0x1000, historyA, stagePreds);
1374-
auto meta = tage->getPredictionMeta();
1375-
1376-
BTBEntry newEntry = createBTBEntry(0x1010, true, true, false, -1);
1377-
FetchTarget stream;
1378-
stream.startPC = 0x1000;
1379-
stream.exeBranchInfo = newEntry;
1380-
stream.exeTaken = true;
1381-
stream.resolved = true;
1382-
stream.predBranchInfo = newEntry;
1383-
stream.updateBTBEntries.clear();
1384-
stream.updateIsOldEntry = false;
1385-
stream.updateNewBTBEntry = newEntry;
1386-
stream.predMetas[0] = meta;
1387-
stream = setMispredStream(stream);
1388-
1389-
tage->update(stream);
1390-
1391-
EXPECT_TRUE(tage->hasExactEntry(0, newEntry.pc, historyA));
1392-
}
1393-
1394-
TEST_F(BTBTAGEUpperBoundPathHashTest, PredictionUsesPathHashHistorySnapshot) {
1395-
BTBEntry entry = createBTBEntry(0x1000, true, true, false, -1, 0x2000);
1396-
boost::dynamic_bitset<> pathHistoryA(128, 0);
1397-
boost::dynamic_bitset<> pathHistoryB(128, 0);
1398-
applyPathHistoryTaken(pathHistoryB, entry.pc, entry.target);
1399-
1400-
ASSERT_TRUE(tage->insertExactEntry(2, entry.pc, pathHistoryB, 2));
1401-
1402-
FullBTBPrediction pred;
1403-
pred.btbEntries.push_back(entry);
1404-
pred.condTakens.push_back({entry.pc, true});
1405-
tage->specUpdatePHist(pathHistoryA, pred);
1406-
1407-
bool predicted = predictTAGE(tage, 0x1000, {entry}, outcomeHistory, stagePreds);
1408-
1409-
EXPECT_TRUE(predicted);
1410-
}
14111212

14121213

14131214
} // namespace test

0 commit comments

Comments
 (0)