Skip to content
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.

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

Change for split requests #257

Open
wants to merge 5 commits into
base: jason/dynamorio-traces
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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: 67 additions & 10 deletions src/cpu/testers/dr_trace_player/trace_player.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,24 +167,53 @@ DRTracePlayer::executeMemInst(DRTraceReader::TraceRef &mem_ref)
bool
DRTracePlayer::trySendMemRef(DRTraceReader::TraceRef &mem_ref)
{
PacketPtr pkt = getPacket(mem_ref);
PacketPtr pkt, split_pkt;

std::tie(pkt, split_pkt) = getPacket(mem_ref);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
PacketPtr pkt, split_pkt;
std::tie(pkt, split_pkt) = getPacket(mem_ref);
auto [pkt, split_pkt] = getPacket(mem_ref);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


if (!retrySplitPkt) {
// we should not be sending the first pkt again if
// we are retrying only on the second part of the
// previously stalled request
Copy link
Member

Choose a reason for hiding this comment

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

This comment is for the other side of the branch, not this side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I have understood correctly, the place of the comment should be changed?

Copy link
Member

Choose a reason for hiding this comment

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

Correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added/fixed comments to hopefully make things better and not worse

DPRINTF(DRTrace, "Trying to send %s\n", pkt->print());

if (!port.sendTimingReq(pkt)) {
DPRINTF(DRTrace, "Failed to send pkt\n");
if (stats.memStallStart == 0) {
stats.memStallStart = curTick();
}
delete pkt;
if (!split_pkt) {
// if this is not a split pkt req,
// we can return back here
return true;
powerjg marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
stats.latencyTracker[pkt] = curTick();
return false;
}
}
powerjg marked this conversation as resolved.
Show resolved Hide resolved

DPRINTF(DRTrace, "Trying to send %s\n", pkt->print());
DPRINTF(DRTrace, "Trying to send split %s\n", split_pkt->print());
powerjg marked this conversation as resolved.
Show resolved Hide resolved

if (!port.sendTimingReq(pkt)) {
DPRINTF(DRTrace, "Failed to send pkt\n");
if (!port.sendTimingReq(split_pkt)) {
DPRINTF(DRTrace, "Failed to send pkt (split pkt) \n");
if (stats.memStallStart == 0) {
stats.memStallStart = curTick();
}
delete pkt;
retrySplitPkt = false;
delete split_pkt;
return true;
} else {
stats.latencyTracker[pkt] = curTick();
stats.latencyTracker[split_pkt] = curTick();
// also remember that we only need to retry on second part of
// the split pkt
retrySplitPkt = true;
powerjg marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
}

PacketPtr
std::pair<PacketPtr, PacketPtr>
Copy link
Member

Choose a reason for hiding this comment

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

I think this is right, but you should double check me :).

Suggested change
std::pair<PacketPtr, PacketPtr>
std::tuple<PacketPtr, PacketPtr>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

DRTracePlayer::getPacket(DRTraceReader::TraceRef &mem_ref)
{
Request::Flags flags = Request::PHYSICAL;
Expand All @@ -198,11 +227,13 @@ DRTracePlayer::getPacket(DRTraceReader::TraceRef &mem_ref)
addr %= compressAddressRange.size();
}

bool split_req = false;
unsigned size = mem_ref.size;
Addr split_addr = roundDown(addr + size - 1, cacheLineSize);
if (split_addr > addr) {
warn("Ignoring split packet that crosses cache line boundary.");
DPRINTF(DRTrace, "Split pkt (crosses cache line boundary) created\n");
size = split_addr - addr;
split_req = true;
}

// Create new request
Expand Down Expand Up @@ -230,9 +261,35 @@ DRTracePlayer::getPacket(DRTraceReader::TraceRef &mem_ref)
}
}

return pkt;
}
PacketPtr split_pkt = NULL;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
PacketPtr split_pkt = NULL;
PacketPtr split_pkt = nullptr;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


// In case of split packets when we want to send two requests.
// For the second request, the starting address
// will be split_addr and the size will be cacheLineSize - size

if (split_req) {

// Create the split request
RequestPtr split_req = std::make_shared<Request>(split_addr,
cacheLineSize - size, flags, requestorId);
split_req->setPC(curPC);

// Embed it in a packet
split_pkt = new Packet(split_req, cmd);

if (params().send_data) {
uint8_t* split_pkt_data = new uint8_t[split_req->getSize()];
split_pkt->dataDynamic(split_pkt_data);

if (cmd.isWrite()) {
std::fill_n(split_pkt_data, split_req->getSize(),
(uint8_t)requestorId);
}
}
}

return std::make_pair(pkt, split_pkt);
Copy link
Member

Choose a reason for hiding this comment

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

I think the following works in modern C++.

Suggested change
return std::make_pair(pkt, split_pkt);
return {pkt, split_pkt};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

Port &
DRTracePlayer::getPort(const std::string &if_name, PortID idx)
Expand Down
6 changes: 5 additions & 1 deletion src/cpu/testers/dr_trace_player/trace_player.hh
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class DRTracePlayer : public ClockedObject
AddrRange compressAddressRange;
int cacheLineSize;

// variable to keep track of retries for split pkts
bool retrySplitPkt = false;

// State
bool stalled = false;
Addr curPC = 0;
Expand Down Expand Up @@ -129,7 +132,8 @@ class DRTracePlayer : public ClockedObject

bool recvTimingResp(PacketPtr pkt);

PacketPtr getPacket(DRTraceReader::TraceRef &mem_ref);
std::pair<PacketPtr, PacketPtr>
getPacket(DRTraceReader::TraceRef &mem_ref);

/**
* @brief Send a request to memory based on the trace
Expand Down