Skip to content

Commit 5fb402c

Browse files
[SYCL] fix incorrect application of binary AND in tests (#16610)
&= with 0 is always 0. For tests that are returning 0 as success, using binary AND will mask errors, leading the test to incorrectly pass when it should fail. I wasted an hour last week being mislead by this problem. I audited all our tests and fortunately the ones making this mistake were relatively few.
1 parent a5e8209 commit 5fb402c

File tree

6 files changed

+29
-29
lines changed

6 files changed

+29
-29
lines changed

sycl/test-e2e/Basic/fpga_tests/fpga_pipes.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -329,26 +329,26 @@ int main() {
329329
}
330330

331331
// Non-blocking pipes
332-
int Result = test_simple_nb_pipe<some_nb_pipe, /*test number*/ 1>(Queue);
333-
Result &= test_simple_nb_pipe<some::nb_pipe, /*test number*/ 2>(Queue);
332+
int Error = test_simple_nb_pipe<some_nb_pipe, /*test number*/ 1>(Queue);
333+
Error |= test_simple_nb_pipe<some::nb_pipe, /*test number*/ 2>(Queue);
334334
class forward_nb_pipe;
335-
Result &= test_simple_nb_pipe<forward_nb_pipe, /*test number*/ 3>(Queue);
336-
Result &= test_simple_nb_pipe<templ_nb_pipe<0>, /*test number*/ 4>(Queue);
337-
Result &= test_multiple_nb_pipe</*test number*/ 5>(Queue);
335+
Error |= test_simple_nb_pipe<forward_nb_pipe, /*test number*/ 3>(Queue);
336+
Error |= test_simple_nb_pipe<templ_nb_pipe<0>, /*test number*/ 4>(Queue);
337+
Error |= test_multiple_nb_pipe</*test number*/ 5>(Queue);
338338

339339
// Blocking pipes
340-
Result &= test_simple_bl_pipe<some_bl_pipe, /*test number*/ 6>(Queue);
341-
Result &= test_simple_bl_pipe<some::bl_pipe, /*test number*/ 7>(Queue);
340+
Error |= test_simple_bl_pipe<some_bl_pipe, /*test number*/ 6>(Queue);
341+
Error |= test_simple_bl_pipe<some::bl_pipe, /*test number*/ 7>(Queue);
342342
class forward_bl_pipe;
343-
Result &= test_simple_bl_pipe<forward_bl_pipe, /*test number*/ 8>(Queue);
344-
Result &= test_simple_bl_pipe<templ_bl_pipe<0>, /*test number*/ 9>(Queue);
345-
Result &= test_multiple_bl_pipe</*test number*/ 10>(Queue);
343+
Error |= test_simple_bl_pipe<forward_bl_pipe, /*test number*/ 8>(Queue);
344+
Error |= test_simple_bl_pipe<templ_bl_pipe<0>, /*test number*/ 9>(Queue);
345+
Error |= test_multiple_bl_pipe</*test number*/ 10>(Queue);
346346

347347
// Test for an array data passing through a pipe
348-
Result &= test_array_th_nb_pipe</*test number*/ 11>(Queue);
349-
Result &= test_array_th_bl_pipe</*test number*/ 12>(Queue);
348+
Error |= test_array_th_nb_pipe</*test number*/ 11>(Queue);
349+
Error |= test_array_th_bl_pipe</*test number*/ 12>(Queue);
350350

351351
// TODO Remove when #14308 is closed
352-
std::cerr << "DEBUG: Finished with result " << Result << std::endl;
353-
return Result;
352+
std::cerr << "DEBUG: Finished with result " << Error << std::endl;
353+
return Error;
354354
}

sycl/test-e2e/Basic/launch_queries/max_num_work_groups.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,8 @@ int main() {
206206

207207
using namespace kernels;
208208

209-
int ret{0};
210-
ret &= test_max_num_work_groups<TestKernel>(q, dev);
211-
ret &= test_max_num_work_groups<TestLocalMemoryKernel>(q, dev);
212-
return ret;
209+
int Error{0};
210+
Error |= test_max_num_work_groups<TestKernel>(q, dev);
211+
Error |= test_max_num_work_groups<TestLocalMemoryKernel>(q, dev);
212+
return Error;
213213
}

sycl/test-e2e/Scheduler/MultipleDevices.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,30 +92,30 @@ int multidevice_test(queue MyQueue1, queue MyQueue2) {
9292

9393
int main() {
9494

95-
int Result = -1;
95+
int Error = 0;
9696
try {
9797
queue MyQueue1(cpu_selector_v);
9898
queue MyQueue2(cpu_selector_v);
99-
Result &= multidevice_test(MyQueue1, MyQueue2);
99+
Error |= multidevice_test(MyQueue1, MyQueue2);
100100
} catch (sycl::exception &) {
101101
std::cout << "Skipping CPU and CPU" << std::endl;
102102
}
103103

104104
try {
105105
queue MyQueue1(cpu_selector_v);
106106
queue MyQueue2(gpu_selector_v);
107-
Result &= multidevice_test(MyQueue1, MyQueue2);
107+
Error |= multidevice_test(MyQueue1, MyQueue2);
108108
} catch (sycl::exception &) {
109109
std::cout << "Skipping CPU and GPU" << std::endl;
110110
}
111111

112112
try {
113113
queue MyQueue1(gpu_selector_v);
114114
queue MyQueue2(gpu_selector_v);
115-
Result &= multidevice_test(MyQueue1, MyQueue2);
115+
Error |= multidevice_test(MyQueue1, MyQueue2);
116116
} catch (sycl::exception &) {
117117
std::cout << "Skipping GPU and GPU" << std::endl;
118118
}
119119

120-
return Result;
120+
return Error;
121121
}

sycl/test-e2e/XPTI/buffer/in_cycle.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ bool func(sycl::queue &Queue, int depth = 0) {
4545
}
4646

4747
if (depth > 0)
48-
MismatchFound &= func(Queue, depth - 1);
48+
MismatchFound |= func(Queue, depth - 1);
4949
return MismatchFound;
5050
}
5151
int main() {
@@ -66,7 +66,7 @@ int main() {
6666
// CHECK:{{[0-9]+}}|Release buffer|[[USERID3]]|[[BEID3]]
6767
// CHECK:{{[0-9]+}}|Destruct buffer|[[USERID3]]
6868
for (int i = 0; i < 3; i++)
69-
MismatchFound &= func(Queue);
69+
MismatchFound |= func(Queue);
7070
return MismatchFound;
7171
}
7272

sycl/test-e2e/XPTI/buffer/recursion.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ int main() {
6565
// CHECK:{{[0-9]+}}|Destruct buffer|[[USERID2]]
6666
// CHECK:{{[0-9]+}}|Release buffer|[[USERID1]]|[[BEID1]]
6767
// CHECK:{{[0-9]+}}|Destruct buffer|[[USERID1]]
68-
MismatchFound &= func(Queue, 2);
68+
MismatchFound |= func(Queue, 2);
6969

7070
return MismatchFound;
7171
}

sycl/test/fpga_tests/fpga_io_pipes.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,10 @@ int main() {
123123
}
124124

125125
// Non-blocking pipes
126-
int Result = test_io_nb_pipe(Queue);
126+
int Error = test_io_nb_pipe(Queue); // 0 if successful
127127

128128
// Blocking pipes
129-
Result &= test_io_bl_pipe(Queue);
129+
Error |= test_io_bl_pipe(Queue);
130130

131-
return Result;
131+
return Error;
132132
}

0 commit comments

Comments
 (0)