Skip to content

Commit f75b439

Browse files
[SYCL] Fix read/writes after discard access mode (#3064)
Fix an issue where whenever a read/write was omitted due to a discard access mode, the current context was not updated for the corresponding memory object record, leading to faulty behaviour.
1 parent d1d6c5d commit f75b439

File tree

3 files changed

+26
-3
lines changed

3 files changed

+26
-3
lines changed

sycl/source/detail/scheduler/graph_builder.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,7 @@ Command *Scheduler::GraphBuilder::insertMemoryMove(MemObjRecord *Record,
355355

356356
if ((Req->MAccessMode == access::mode::discard_write) ||
357357
(Req->MAccessMode == access::mode::discard_read_write)) {
358+
Record->MCurContext = Queue->getContextImplPtr();
358359
return nullptr;
359360
} else {
360361
// Full copy of buffer is needed to avoid loss of data that may be caused

sycl/unittests/scheduler/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ add_sycl_unittest(SchedulerTests OBJECT
88
WaitAfterCleanup.cpp
99
LinkedAllocaDependencies.cpp
1010
LeavesCollection.cpp
11-
NoUnifiedHostMemory.cpp
11+
NoHostUnifiedMemory.cpp
1212
StreamInitDependencyOnHost.cpp
1313
InOrderQueueDeps.cpp
1414
utils.cpp

sycl/unittests/scheduler/NoUnifiedHostMemory.cpp renamed to sycl/unittests/scheduler/NoHostUnifiedMemory.cpp

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//==----------- NoUnifiedHostMemory.cpp --- Scheduler unit tests -----------==//
1+
//==----------- NoHostUnifiedMemory.cpp --- Scheduler unit tests -----------==//
22
//
33
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
44
// See https://llvm.org/LICENSE.txt for license information.
@@ -62,7 +62,7 @@ static pi_result redefinedEnqueueMemBufferWriteRect(
6262

6363
static pi_result redefinedMemRelease(pi_mem mem) { return PI_SUCCESS; }
6464

65-
TEST_F(SchedulerTest, NoUnifiedHostMemory) {
65+
TEST_F(SchedulerTest, NoHostUnifiedMemory) {
6666
platform Plt{default_selector()};
6767
if (Plt.is_host()) {
6868
std::cout << "Not run due to host-only environment\n";
@@ -144,4 +144,26 @@ TEST_F(SchedulerTest, NoUnifiedHostMemory) {
144144
detail::Command *MemoryMove = MS.insertMemoryMove(Record, &Req, QImpl);
145145
EXPECT_EQ(MemoryMove->getType(), detail::Command::COPY_MEMORY);
146146
}
147+
// Check that memory movement operations work correctly with/after discard
148+
// access modes.
149+
{
150+
int val;
151+
buffer<int, 1> Buf(&val, range<1>(1));
152+
detail::Requirement Req = getMockRequirement(Buf);
153+
ExpectedMemObjFlags = PI_MEM_FLAGS_ACCESS_RW | PI_MEM_FLAGS_HOST_PTR_COPY;
154+
155+
detail::Requirement DiscardReq = getMockRequirement(Buf);
156+
DiscardReq.MAccessMode = access::mode::discard_read_write;
157+
158+
detail::MemObjRecord *Record = MS.getOrInsertMemObjRecord(QImpl, &Req);
159+
MS.getOrCreateAllocaForReq(Record, &Req, QImpl);
160+
MS.getOrCreateAllocaForReq(Record, &Req, DefaultHostQueue);
161+
162+
// Memory movement operations should be omitted for discard access modes.
163+
detail::Command *MemoryMove =
164+
MS.insertMemoryMove(Record, &DiscardReq, DefaultHostQueue);
165+
EXPECT_EQ(MemoryMove, nullptr);
166+
// The current context for the record should still be modified.
167+
EXPECT_EQ(Record->MCurContext, DefaultHostQueue->getContextImplPtr());
168+
}
147169
}

0 commit comments

Comments
 (0)