Skip to content

wip #706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tastynoob wants to merge 1 commit into
base: xs-dev
Choose a base branch
from
Open

wip #706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 37 additions & 40 deletions src/cpu/o3/issue_queue.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@
selector->deallocate(x); \
} while (0)

#define READYQ_PUSH(x) \
do { \
(x)->setInReadyQ(); \
auto& readyQ = readyQclassify[(x)->opClass()]; \
auto it = std::lower_bound(readyQ->begin(), readyQ->end(), (x), select_policy()); \
readyQ->insert(it, (x)); \
} while (0)

// must be consistent with FUScheduler.py
// rfTypePortId = regfile typeid + portid
#define MAXVAL_TYPEPORTID (1 << (2 + 4)) // [5:4] is typeid, [3:0] is portid
Expand Down Expand Up @@ -191,21 +183,18 @@ IssueQue::IssueQue(const IssueQueParams& params)
iqsize(params.size),
scheduleToExecDelay(params.scheduleToExecDelay),
iqname(params.name),
inflightIssues(scheduleToExecDelay, 0),
selector(params.sel)
{
toIssue = inflightIssues.getWire(0);
toFu = inflightIssues.getWire(-scheduleToExecDelay);
if (outports > 8) {
panic("%s: outports > 8 is not supported\n", iqname);
}
toIssue.resize(outports);
toFu.resize(outports);

portBusy.resize(outports, 0);

intRdRfTPI.resize(outports);
fpRdRfTPI.resize(outports);
intWrRfTPI.resize(outports);

readyQs.resize(outports, nullptr);

readyQclassify.resize(Num_OpClasses, nullptr);
Expand Down Expand Up @@ -355,7 +344,6 @@ IssueQue::addToFu(const DynInstPtr& inst)
void
IssueQue::issueToFu()
{
int size = toFu->size;
int replayed = 0;
int issued = 0;

Expand Down Expand Up @@ -393,8 +381,8 @@ IssueQue::issueToFu()
issued++;
}

for (int i = 0; i < size; i++) {
auto inst = toFu->pop();
for (int i = 0; i < outports; i++) {
auto& inst = *toFu[i];
if (!inst) {
continue;
}
Expand All @@ -409,7 +397,7 @@ IssueQue::issueToFu()
(inst->isStore() && (issuedStore >= numStorePipe)) || blockLoad) {
inst->clearScheduled();
// only for load/store
READYQ_PUSH(inst);
readyQInsert(inst);
DPRINTF(Schedule, "[sn:%llu] issue failed due to being occupied\n", inst->seqNum);
continue;
}
Expand Down Expand Up @@ -526,7 +514,7 @@ IssueQue::addIfReady(const DynInstPtr& inst)
DPRINTF(Schedule, "[sn:%llu] add to readyInstsQue\n", inst->seqNum);
inst->clearCancel();
if (!inst->inReadyQ()) {
READYQ_PUSH(inst);
readyQInsert(inst);
}
}
}
Expand Down Expand Up @@ -629,12 +617,12 @@ IssueQue::scheduleInst()
iqstats->arbFailed++;
assert(inst->readyToIssue());

READYQ_PUSH(inst);
readyQInsert(inst);
} else [[likely]] {
DPRINTF(Schedule, "[sn:%llu] no conflict, scheduled\n", inst->seqNum);
iqstats->portissued[pi]++;
inst->setScheduled();
toIssue->push(inst);
*toIssue[pi] = inst;
inst->issueportid = pi;

if (!opPipelined[inst->opClass()]) {
Expand All @@ -661,7 +649,6 @@ IssueQue::tick()
instNumInsert = 0;

scheduleInst();
inflightIssues.advance();

for (auto& t : portBusy) {
t = t >> 1;
Expand Down Expand Up @@ -774,16 +761,6 @@ IssueQue::doSquash(const InstSeqNum seqNum)
}
}

for (int i = 0; i <= getIssueStages(); i++) {
int size = inflightIssues[-i].size;
for (int j = 0; j < size; j++) {
auto& inst = inflightIssues[-i].insts[j];
if (inst && inst->isSquashed()) {
inst = nullptr;
}
}
}

// clear in depGraph
for (auto& entrys : subDepGraph) {
for (auto it = entrys.begin(); it != entrys.end();) {
Expand Down Expand Up @@ -853,10 +830,13 @@ Scheduler::Scheduler(const SchedulerParams& params)
int maxRdTypePortId = 0;
int maxWrTypePortId = 0;
for (int i = 0; i < issueQues.size(); i++) {
auto iq = issueQues[i];
issueQues[i]->setIQID(i);
issueQues[i]->scheduler = this;
combinedFus += issueQues[i]->outports;
panic_if(issueQues[i]->fuDescs.size() == 0, "Empty config IssueQue: " + issueQues[i]->getName());
for (int j = 0; j < iq->outports; j++) {
inflightIssues.push_back(TimeBuffer<DynInstPtr>(0, iq->scheduleToExecDelay));
}
for (auto fu : issueQues[i]->fuDescs) {
for (auto op : fu->opDescList) {
opExecTimeTable[op->opClass] = op->opLat;
Expand Down Expand Up @@ -899,6 +879,13 @@ Scheduler::Scheduler(const SchedulerParams& params)
}
wrRfPortOccupancy.resize(maxWrTypePortId, {nullptr, 0, 0});

int portid = 0;
for (auto iq : issueQues) {
for (int i = 0; i < iq->outports; i++) {
iq->setIssuePipe(inflightIssues[portid], i);
portid++;
}
}

// dispatch distance counter allocate
dispOpdist.resize(Num_OpClasses, nullptr);
Expand Down Expand Up @@ -1027,6 +1014,10 @@ Scheduler::tick()
for (auto it : issueQues) {
it->tick();
}

for (auto& it : inflightIssues) {
it.advance();
}
}

void
Expand Down Expand Up @@ -1392,14 +1383,11 @@ Scheduler::loadCancel(const DynInstPtr& inst)
}
}

for (auto iq : issueQues) {
for (int i = 0; i <= iq->getIssueStages(); i++) {
int size = iq->inflightIssues[-i].size;
for (int j = 0; j < size; j++) {
auto& inst = iq->inflightIssues[-i].insts[j];
if (inst && inst->canceled()) {
inst = nullptr;
}
for (auto& it : inflightIssues) {
for (int i = 0; i < it.getSize(); i++) {
auto& inst = it[i];
if (inst && inst->canceled()) {
inst = nullptr;
}
}
}
Expand Down Expand Up @@ -1500,6 +1488,15 @@ Scheduler::doSquash(const InstSeqNum seqNum)
for (auto it : issueQues) {
it->doSquash(seqNum);
}

for (auto& it : inflightIssues) {
for (int i = 0; i < it.getSize(); i++) {
auto& inst = it[i];
if (inst && inst->isSquashed()) {
inst = nullptr;
}
}
}
}

uint32_t
Expand Down
23 changes: 18 additions & 5 deletions src/cpu/o3/issue_queue.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,15 @@ class IssueQue : public SimObject
friend class PAgeSelector;

std::string _name;
public:
const int inports;
const int outports;
const int iqsize;
const int replayQsize = 32;
const int scheduleToExecDelay;
const std::string iqname;

private:
std::vector<std::bitset<Num_OpClasses>> portFuDescs;
std::vector<FUDesc*> fuDescs;
std::vector<bool> opPipelined;
Expand All @@ -133,10 +136,8 @@ class IssueQue : public SimObject
DynInstPtr pop();
};

std::vector<DynInstPtr> skidBuffer;
TimeBuffer<IssueStream> inflightIssues;
TimeBuffer<IssueStream>::wire toIssue;
TimeBuffer<IssueStream>::wire toFu;
std::vector<TimeBuffer<DynInstPtr>::wire> toIssue;
std::vector<TimeBuffer<DynInstPtr>::wire> toFu;

std::list<DynInstPtr> instList;
uint64_t instNumInsert = 0;
Expand Down Expand Up @@ -196,9 +197,20 @@ class IssueQue : public SimObject
void scheduleInst();
void addIfReady(const DynInstPtr& inst);
void cancel(const DynInstPtr& inst);
inline void readyQInsert(const DynInstPtr& x) {
x->setInReadyQ();
auto& readyQ = readyQclassify[x->opClass()];
auto it = std::lower_bound(readyQ->begin(), readyQ->end(), x, select_policy());
readyQ->insert(it, x);
}


public:
inline void clearBusy(uint32_t pi) { portBusy.at(pi) = 0; }
inline void setIssuePipe(TimeBuffer<DynInstPtr>& issuepipe, int pi) {
toIssue[pi] = issuepipe.getWire(scheduleToExecDelay);
toFu[pi] = issuepipe.getWire(0);
}
Comment on lines +210 to +213
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing bounds check in setIssuePipe.

The method accesses toIssue[pi] and toFu[pi] without validating that pi is within bounds. If called with an invalid port index, this will cause undefined behavior.

🐛 Proposed fix to add bounds checking 
     inline void setIssuePipe(TimeBuffer<DynInstPtr>& issuepipe, int pi) {
+      assert(pi >= 0 && pi < outports);
       toIssue[pi] = issuepipe.getWire(scheduleToExecDelay);
       toFu[pi] = issuepipe.getWire(0);
     }
🤖 Prompt for AI Agents 
In `@src/cpu/o3/issue_queue.hh` around lines 210 - 213, setIssuePipe currently
writes to toIssue[pi] and toFu[pi] without validating pi; add a bounds check at
the start of setIssuePipe to ensure 0 <= pi < N (where N is the size/capacity of
toIssue/toFu) and early-return or assert/log an error if out of range, then only
call issuepipe.getWire(scheduleToExecDelay) and issuepipe.getWire(0) when pi is
valid; reference setIssuePipe, toIssue, toFu, issuepipe.getWire, and
scheduleToExecDelay when implementing the check.


IssueQue(const IssueQueParams& params);
void setIQID(int id) { IQID = id; }
Expand Down Expand Up @@ -288,11 +300,12 @@ class Scheduler : public SimObject
std::vector<IQGroup> dispTable;
std::vector<IssueQue*> issueQues;
std::vector<std::vector<IssueQue*>> wakeMatrix;
uint32_t combinedFus;

std::vector<uint8_t> totalDispCounter;
std::vector<uint8_t*> dispOpdist;

// Centralized management
std::vector<TimeBuffer<DynInstPtr>> inflightIssues;
std::vector<DynInstPtr> instsToFu;

std::vector<bool> earlyScoreboard;
Expand Down
59 changes: 32 additions & 27 deletions src/cpu/timebuf.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,12 @@ template <class T>
class TimeBuffer
{
protected:
int past;
int future;
unsigned size;
int _id;

char *data;
std::vector<char *> index;
unsigned base;
int _id = -1;
int past = 0;
int future = 0;
unsigned size = 0;
unsigned base = 0;
T* datas = nullptr;

void valid(int idx) const
{
Expand Down Expand Up @@ -138,31 +136,38 @@ class TimeBuffer

public:
TimeBuffer(int p, int f)
: past(p), future(f), size(past + future + 1),
data(new char[size * sizeof(T)]), index(size), base(0)
: past(p), future(f), size(past + future + 1)
{
assert(past >= 0 && future >= 0);
char *ptr = data;
datas = (T*)new char[sizeof(T) * size];
std::memset((void*)datas, 0, sizeof(T) * size);
for (unsigned i = 0; i < size; i++) {
index[i] = ptr;
std::memset(ptr, 0, sizeof(T));
new (ptr) T;
ptr += sizeof(T);
new (datas + i) T;
}

_id = -1;
}

TimeBuffer()
: data(NULL)
TimeBuffer() {}

TimeBuffer(const TimeBuffer<T> &other)
: _id(other._id), past(other.past), future(other.future), size(other.size), base(other.base)
{
datas = new T[size];
for (unsigned i = 0; i < size; i++) {
datas[i] = other.datas[i]; // must use explicit copy to handle non-POD types
}
}

TimeBuffer(TimeBuffer<T> &&other) noexcept
: _id(other._id), past(other.past), future(other.future), size(other.size), base(other.base), datas(other.datas)
{
// Null out the other datas pointer to avoid double deletion
other.datas = nullptr;
}

~TimeBuffer()
{
for (unsigned i = 0; i < size; ++i)
(reinterpret_cast<T *>(index[i]))->~T();
delete [] data;
delete [] datas;
Comment on lines +150 to +170
Copy link

@coderabbitai coderabbitai bot Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing copy and move assignment operators violates Rule of 5.

The class defines a destructor, copy constructor, and move constructor, but lacks copy assignment and move assignment operators. This can lead to:

  1. Double-free if default copy assignment is used
  2. Resource leaks if default move assignment is used
🐛 Proposed addition of assignment operators 
TimeBuffer& operator=(const TimeBuffer<T>& other)
{
    if (this != &other) {
        // Clean up existing resources
        if (datas) {
            for (unsigned i = 0; i < size; i++) {
                datas[i].~T();
            }
            delete[] reinterpret_cast<char*>(datas);
        }
        
        // Copy from other
        _id = other._id;
        past = other.past;
        future = other.future;
        size = other.size;
        base = other.base;
        
        if (size > 0) {
            datas = (T*)new char[sizeof(T) * size];
            for (unsigned i = 0; i < size; i++) {
                new (datas + i) T(other.datas[i]);
            }
        } else {
            datas = nullptr;
        }
    }
    return *this;
}

TimeBuffer& operator=(TimeBuffer<T>&& other) noexcept
{
    if (this != &other) {
        // Clean up existing resources
        if (datas) {
            for (unsigned i = 0; i < size; i++) {
                datas[i].~T();
            }
            delete[] reinterpret_cast<char*>(datas);
        }
        
        // Move from other
        _id = other._id;
        past = other.past;
        future = other.future;
        size = other.size;
        base = other.base;
        datas = other.datas;
        
        other.datas = nullptr;
        other.size = 0;
    }
    return *this;
}
🤖 Prompt for AI Agents 
In @src/cpu/timebuf.hh around lines 150 - 170, The class TimeBuffer defines a
destructor, copy ctor and move ctor but lacks copy and move assignment
operators; implement TimeBuffer& operator=(const TimeBuffer<T>&) and TimeBuffer&
operator=(TimeBuffer<T>&&) to satisfy the Rule of 5: in copy-assignment guard
against self-assignment, free existing resources held in datas, deep-copy
other's metadata (_id, past, future, size, base) and allocate/copy elements into
datas (handling non-POD via placement-new or proper element copy), and in
move-assignment guard self-assignment, free existing resources, transfer
metadata and datas pointer from other to this, set other.datas = nullptr and
other.size = 0, and return *this; ensure consistency with the existing
destructor and move/copy ctors to avoid double-free or leaks.

}

void id(int id)
Expand All @@ -184,9 +189,9 @@ class TimeBuffer
int ptr = base + future;
if (ptr >= (int)size)
ptr -= size;
(reinterpret_cast<T *>(index[ptr]))->~T();
std::memset(index[ptr], 0, sizeof(T));
new (index[ptr]) T;
datas[ptr].~T();
std::memset((void*)(datas + ptr), 0, sizeof(T));
new (datas + ptr) T;
}

protected:
Expand All @@ -212,21 +217,21 @@ class TimeBuffer
{
int vector_index = calculateVectorIndex(idx);

return reinterpret_cast<T *>(index[vector_index]);
return datas + vector_index;
}

T &operator[](int idx)
{
int vector_index = calculateVectorIndex(idx);

return reinterpret_cast<T &>(*index[vector_index]);
return datas[vector_index];
}

const T &operator[] (int idx) const
{
int vector_index = calculateVectorIndex(idx);

return reinterpret_cast<const T &>(*index[vector_index]);
return datas[vector_index];
}

wire getWire(int idx)
Expand Down