Stage two refactor to cache device offline mode setting

sophimao · sophimao · commit 169df383f067 · 2023-01-09T08:10:52.000-08:00
diff --git a/include/acl_globals.h b/include/acl_globals.h
@@ -44,18 +44,8 @@ void acl_reset(void);
 // if it is not.
 cl_bool acl_init_from_hal_discovery(void);
 
-// Looks at environment variable CL_CONTEXT_OFFLINE_DEVICE_INTELFPGA.
-// If it exists and is prefixed by "+" then:
-//    Return a pointer to the device name (without the "+" prefix).
-//    Set *use_offline_ret_only = 1
-// If it exists and is not prefixed by "+" then
-//    Return a pointer to the device name.
-//    Set *use_offline_ret_only = 0
-#define ACL_CONTEXT_OFFLINE_AND_AUTODISCOVERY 0
-#define ACL_CONTEXT_OFFLINE_ONLY 1
-#define ACL_CONTEXT_MSIM 3
-#define ACL_CONTEXT_MPSIM 4
-const char *acl_get_offline_device_user_setting(int *use_offline_only_ret);
+acl_context_offline_mode_t
+acl_get_offline_device_user_setting(std::string *offline_device);
 
 ACL_EXPORT
 extern struct _cl_platform_id acl_platform;
diff --git a/include/acl_types.h b/include/acl_types.h
@@ -216,6 +216,20 @@ typedef enum {
   ACL_COMPILER_MODE_NUM_MODES = CL_CONTEXT_COMPILER_MODE_NUM_MODES_INTELFPGA
 } acl_compiler_mode_t;
 
+// Looks at environment variable CL_CONTEXT_OFFLINE_DEVICE_INTELFPGA.
+// If it exists and is prefixed by "+" then:
+//    Return a pointer to the device name (without the "+" prefix).
+//    Set *use_offline_ret_only = 1
+// If it exists and is not prefixed by "+" then
+//    Return a pointer to the device name.
+//    Set *use_offline_ret_only = 0
+typedef enum {
+  ACL_CONTEXT_OFFLINE_AND_AUTODISCOVERY,
+  ACL_CONTEXT_OFFLINE_ONLY,
+  ACL_CONTEXT_MSIM,
+  ACL_CONTEXT_MPSIM
+} acl_context_offline_mode_t;
+
 /* When a feature is still in development, it might need enum values
  * that are distinct from all published enums in the OpenCL token registry.
  * There is a reserved range for values for such "experimental" features.
@@ -1226,7 +1240,8 @@ typedef struct _cl_device_id {
 
   unsigned int address_bits; // cache address bits to avoid GetDeviceInfo calls
 
-  int present; // Is the device present in the host system?
+  bool present; // Is the device present in the host system?
+  bool offline; // Is the device a real (i.e., not simulator) offline device?
 
   // Error notification callback.
   CL_EXCEPTION_TYPE_INTEL device_exception_status;
@@ -1573,13 +1588,16 @@ typedef struct _cl_platform_id
   int device_exception_platform_counter; // indicates number of devices with at
                                          // least one exception
 
-  // The setting of environment variable CL_CONTEXT_OFFLINE_DEVICE_INTELFPGA, if
-  // any.
-  std::string offline_device;
+  // Record whether the platform has (non-simulator) offline device,
+  // value will be:
+  // 1: if there is a valid offline device specified by environment variable
+  // 0: if there is no offline device specified by environment variable
+  // -1: if the offline device specified by environment variable is invalid
+  int has_offline_device;
   // Cache context offline mode specified by environment variables
   // CL_CONTEXT_OFFLINE_DEVICE_INTELFPGA, CL_CONTEXT_MPSIM_DEVICE_INTELFPGA
   // or CL_CONTEXT_MSIM_DEVICE_INTELFPGA
-  int offline_mode;
+  acl_context_offline_mode_t offline_mode;
 
   // Should we track and automatically release leaked objects?
   // This helps immensely with the OpenCL conformance tests which tend to
diff --git a/src/acl_context.cpp b/src/acl_context.cpp
@@ -48,7 +48,6 @@ static void
 l_init_kernel_invocation_wrapper(acl_kernel_invocation_wrapper_t *wrapper,
                                  unsigned i);
 static void l_forcibly_release_allocations(cl_context context);
-static cl_device_id l_find_device_by_name(const std::string &name);
 static cl_int l_update_program_library_root(cl_context context,
                                             const char *new_root);
 static cl_int l_update_compile_command(cl_context context, const char *new_cmd);
@@ -606,12 +605,11 @@ static cl_int l_load_properties(cl_context context,
     }
   }
 
-  // Environment variable can specify we always an offline device.
-  if (!acl_platform.offline_device.empty()) {
-    if (!l_find_device_by_name(acl_platform.offline_device))
-      ERR_RET(CL_INVALID_VALUE, context,
-              "Invalid offline device specified by environment variable "
-              "CL_CONTEXT_OFFLINE_DEVICE_INTELFPGA");
+  // Check if environment variable specified offline device is valid
+  if (acl_platform.has_offline_device < 0) {
+    ERR_RET(CL_INVALID_VALUE, context,
+            "Invalid offline device specified by environment variable "
+            "CL_CONTEXT_OFFLINE_DEVICE_INTELFPGA");
   }
 
   // Get default for program_library_root.
@@ -817,17 +815,6 @@ static cl_int l_load_properties(cl_context context,
   return CL_SUCCESS;
 }
 
-static cl_device_id l_find_device_by_name(const std::string &name) {
-  acl_assert_locked();
-
-  for (unsigned i = 0; i < acl_platform.num_devices; ++i) {
-    if (name == acl_platform.device[i].def.autodiscovery_def.name) {
-      return &(acl_platform.device[i]);
-    }
-  }
-  return 0;
-}
-
 // Initialize the given context.
 // Yes, this is like a "placement new".
 //
@@ -883,8 +870,6 @@ static cl_int l_init_context_with_devices(cl_context context,
   int num_present = 0;
   int num_absent = 0;
   for (cl_uint i = 0; i < num_devices; i++) {
-    int usable = devices[i]->present;
-
     // Can't mix both (actually) present and absent devices because there
     // is no consistent way to place device global memory.
     if (devices[i]->present) {
@@ -900,10 +885,7 @@ static cl_int l_init_context_with_devices(cl_context context,
               "Can't create a context with both offline and online devices");
     }
 
-    usable = usable || acl_platform.offline_device ==
-                           devices[i]->def.autodiscovery_def.name;
-
-    if (!usable)
+    if (!devices[i]->present && !devices[i]->offline)
       ERR_RET(CL_DEVICE_NOT_AVAILABLE, context, "Device not available");
 
     // Mark the device(s) as opened
diff --git a/src/acl_globals.cpp b/src/acl_globals.cpp
@@ -73,8 +73,10 @@ static void l_reset_present_board() {
 // If it's prefixed by "+", then it's in addition to any auto-discovered
 // devices.
 // If not, then we don't even probe for auto-discovered devices.
-const char *acl_get_offline_device_user_setting(int *use_offline_only_ret) {
-  int use_offline_only = 0;
+acl_context_offline_mode_t
+acl_get_offline_device_user_setting(std::string *offline_device) {
+  acl_context_offline_mode_t use_offline_only =
+      ACL_CONTEXT_OFFLINE_AND_AUTODISCOVERY;
   const char *setting = 0;
   const char *setting_deprecated = 0;
   const char *result = 0;
@@ -136,8 +138,15 @@ const char *acl_get_offline_device_user_setting(int *use_offline_only_ret) {
     }
   }
 
-  *use_offline_only_ret = use_offline_only;
-  return result;
+  if (offline_device) {
+    if (use_offline_only == ACL_CONTEXT_MPSIM) {
+      *offline_device = ACL_MPSIM_DEVICE_NAME;
+    } else if (result) {
+      *offline_device = result;
+    }
+  }
+
+  return use_offline_only;
 }
 
 int acl_init(const acl_system_def_t *newsys) {
@@ -166,11 +175,11 @@ int acl_init(const acl_system_def_t *newsys) {
 // This function returns CL_TRUE if a hal is initialized and CL_FALSE
 // if it is not.
 cl_bool acl_init_from_hal_discovery(void) {
-  int use_offline_only = 0;
   const acl_hal_t *board_hal;
   acl_assert_locked();
 
-  (void)acl_get_offline_device_user_setting(&use_offline_only);
+  acl_context_offline_mode_t use_offline_only =
+      acl_get_offline_device_user_setting(NULL);
 
   // Two jobs:
   // 1. Set the HAL from the linked-in HAL library.
@@ -223,8 +232,8 @@ void acl_reset(void) {
 
   l_reset_present_board();
 
-  acl_platform.offline_device = "";
   acl_platform.offline_mode = ACL_CONTEXT_OFFLINE_AND_AUTODISCOVERY;
+  acl_platform.has_offline_device = 0;
   acl_platform.num_devices = 0;
   for (unsigned i = 0; i < ACL_MAX_DEVICE; ++i) {
     acl_platform.device[i] = _cl_device_id();
diff --git a/src/acl_hal_mmd.cpp b/src/acl_hal_mmd.cpp
@@ -1199,7 +1199,6 @@ acl_mmd_get_system_definition(acl_system_def_t *sys,
       buf[MAX_BOARD_NAMES_LEN]; // This is a bit subtle, pointers to device
                                 // names might get cached by various routines
   char *ptr, *saveptr;
-  int use_offline_only;
 
 #ifdef _WIN32
   // We're really relying on this being called before anything else
@@ -1248,7 +1247,8 @@ acl_mmd_get_system_definition(acl_system_def_t *sys,
 #endif
 
   // Dynamically load board mmd & symbols
-  (void)acl_get_offline_device_user_setting(&use_offline_only);
+  acl_context_offline_mode_t use_offline_only =
+      acl_get_offline_device_user_setting(NULL);
   if (use_offline_only == ACL_CONTEXT_MPSIM) {
 
     // Substitute the simulator MMD layer.
diff --git a/src/acl_platform.cpp b/src/acl_platform.cpp
diff --git a/test/acl_command_queue_test.cpp b/test/acl_command_queue_test.cpp
