Addressed concern regarding #494

Added logic to verify that all bits of property integer were recognized and used. Added test to verify handling of property bits.
IntelPython · Aug 18, 2021 · cecde73 · cecde73
1 parent 9197366
commit cecde73
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 5 deletions.
diff --git a/dpctl-capi/include/dpctl_sycl_enum_types.h b/dpctl-capi/include/dpctl_sycl_enum_types.h
@@ -146,8 +146,8 @@ typedef enum
 {
     // clang-format off
     DPCTL_DEFAULT_PROPERTY = 0,
-    DPCTL_ENABLE_PROFILING = 1 << 1,
-    DPCTL_IN_ORDER         = 1 << 2
+    DPCTL_ENABLE_PROFILING = 1 << 0,
+    DPCTL_IN_ORDER         = 1 << 1
     // clang-format on
 } DPCTLQueuePropertyType;
 

diff --git a/dpctl-capi/source/dpctl_sycl_queue_interface.cpp b/dpctl-capi/source/dpctl_sycl_queue_interface.cpp
@@ -119,8 +119,11 @@ bool set_kernel_arg(handler &cgh,
 std::unique_ptr<property_list> create_property_list(int properties)
 {
     std::unique_ptr<property_list> propList;
-    if (properties & DPCTL_ENABLE_PROFILING) {
-        if (properties & DPCTL_IN_ORDER) {
+    int _prop = properties;
+    if (_prop & DPCTL_ENABLE_PROFILING) {
+        _prop = _prop ^ DPCTL_ENABLE_PROFILING;
+        if (_prop & DPCTL_IN_ORDER) {
+            _prop = _prop ^ DPCTL_IN_ORDER;
             propList = std::make_unique<property_list>(
                 sycl::property::queue::enable_profiling(),
                 sycl::property::queue::in_order());
@@ -130,11 +133,18 @@ std::unique_ptr<property_list> create_property_list(int properties)
                 sycl::property::queue::enable_profiling());
         }
     }
-    else if (properties & DPCTL_IN_ORDER) {
+    else if (_prop & DPCTL_IN_ORDER) {
+        _prop = _prop ^ DPCTL_IN_ORDER;
         propList =
             std::make_unique<property_list>(sycl::property::queue::in_order());
     }
 
+    if (_prop) {
+        // todo: log error
+        std::cerr << "Invalid queue property argument (" << std::hex
+                  << properties << "), interpreted as (" << (properties ^ _prop)
+                  << ")" << '\n';
+    }
     return propList;
 }
 

diff --git a/dpctl-capi/tests/test_sycl_queue_interface.cpp b/dpctl-capi/tests/test_sycl_queue_interface.cpp
@@ -292,6 +292,41 @@ TEST(TestDPCTLSyclQueueInterface, CheckHasEnableProfilingInvalid)
     EXPECT_FALSE(ioq);
 }
 
+TEST(TestDPCTLSyclQueueInterface, CheckPropertyHandling)
+{
+    DPCTLSyclQueueRef QRef = nullptr;
+    DPCTLSyclDeviceSelectorRef DSRef = nullptr;
+    DPCTLSyclDeviceRef DRef = nullptr;
+
+    EXPECT_NO_FATAL_FAILURE(DSRef = DPCTLDefaultSelector_Create());
+    EXPECT_NO_FATAL_FAILURE(DRef = DPCTLDevice_CreateFromSelector(DSRef));
+
+    ::testing::internal::CaptureStderr();
+    EXPECT_NO_FATAL_FAILURE(QRef = DPCTLQueue_CreateForDevice(
+                                DRef, nullptr, DPCTL_DEFAULT_PROPERTY));
+    std::string capt1 = ::testing::internal::GetCapturedStderr();
+    ASSERT_TRUE(capt1.empty());
+    ASSERT_FALSE(DPCTLQueue_IsInOrder(QRef));
+    ASSERT_FALSE(DPCTLQueue_HasEnableProfiling(QRef));
+    EXPECT_NO_FATAL_FAILURE(DPCTLQueue_Delete(QRef));
+
+    QRef = nullptr;
+    ::testing::internal::CaptureStderr();
+    int invalid_prop = -1;
+    EXPECT_NO_FATAL_FAILURE(
+        QRef = DPCTLQueue_CreateForDevice(DRef, nullptr, invalid_prop));
+    std::string capt2 = ::testing::internal::GetCapturedStderr();
+    ASSERT_TRUE(!capt2.empty());
+    ASSERT_TRUE(DPCTLQueue_IsInOrder(QRef) ==
+                bool((invalid_prop & DPCTL_IN_ORDER)));
+    ASSERT_TRUE(DPCTLQueue_HasEnableProfiling(QRef) ==
+                bool((invalid_prop & DPCTL_ENABLE_PROFILING)));
+    EXPECT_NO_FATAL_FAILURE(DPCTLQueue_Delete(QRef));
+
+    EXPECT_NO_FATAL_FAILURE(DPCTLDevice_Delete(DRef));
+    EXPECT_NO_FATAL_FAILURE(DPCTLDeviceSelector_Delete(DSRef));
+}
+
 TEST_P(TestDPCTLQueueMemberFunctions, CheckGetBackend)
 {
     auto q = unwrap(QRef);