[Comgr][v3.0] Remove deprecated set/get options APIs

lamb-j · lamb-j · commit 11e989c1e2ba · 2025-01-03T12:10:30.000-05:00
- amd_comgr_action_info_set_options()
  - amd_comgr_action_info_get_options()

This also removes the internal representation of a flat list

Change-Id: Ia35d0f890099d116dd7b3389413eec88baba66f7
diff --git a/amd/comgr/docs/ReleaseNotes.md b/amd/comgr/docs/ReleaseNotes.md
@@ -123,6 +123,11 @@ Deprecated APIs
 
 Removed APIs
 ------------
+- amd\_comgr\_action\_info\_set\_options() (v3.0)
+- amd\_comgr\_action\_info\_get\_options() (v3.0)
+  - Use  amd\_comgr\_action\_info\_set\_option\_list(),
+    amd\_comgr\_action\_info\_get\_option\_list\_count(), and
+    amd\_comgr\_action\_info\_get\_option\_list\_item() instead
 
 New Comgr Actions and Data Types
 --------------------------------
diff --git a/amd/comgr/include/amd_comgr.h.in b/amd/comgr/include/amd_comgr.h.in
@@ -1233,92 +1233,11 @@ amd_comgr_action_info_get_language(
   amd_comgr_action_info_t action_info,
   amd_comgr_language_t *language) AMD_COMGR_VERSION_1_8;
 
-/**
- * @brief Set the options string of an action info object.
- *
- * When an action info object is created it has an empty options string.
- *
- * This overrides any option strings or arrays previously set by calls to this
- * function or @p amd_comgr_action_info_set_option_list.
- *
- * An @p action_info object which had its options set with this function can
- * only have its option inspected with @p amd_comgr_action_info_get_options.
- *
- * @param[in] action_info A handle to the action info object to be
- * updated.
- *
- * @param[in] options A null terminated string that is the options. If
- * NULL or the empty string then the options are cleared.
- *
- * @retval ::AMD_COMGR_STATUS_SUCCESS The function has
- * been executed successfully.
- *
- * @retval ::AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT @p
- * action_info is an invalid action info object.
- *
- * @retval ::AMD_COMGR_STATUS_ERROR_OUT_OF_RESOURCES
- * Unable to update action info object as out of resources.
- *
- * @deprecated since 1.3
- * @see amd_comgr_action_info_set_option_list
- */
-AMD_COMGR_DEPRECATED("Will be removed in Comgr v3.0 (Rocm v6.0). Use "
-                     "amd_comgr_action_info_set_option_list() instead")
-amd_comgr_status_t AMD_COMGR_API
-amd_comgr_action_info_set_options(
-  amd_comgr_action_info_t action_info,
-  const char *options) AMD_COMGR_VERSION_1_8;
-
-/**
- * @brief Get the options string and/or options strings length of an action
- * info object.
- *
- * The @p action_info object must have had its options set with @p
- * amd_comgr_action_info_set_options.
- *
- * @param[in] action_info The action info object to query.
- *
- * @param[in, out] size On entry, the size of @p options. On return, if @p
- * options is NULL, set to the size of the options including the terminating
- * null character.
- *
- * @param[out] options If not NULL, then the first @p size characters of
- * the options are copied. If the options are not set then an empty
- * string is copied. If NULL, options is not copied, and only @p size
- * is updated (useful inorder to find the size of buffer required to
- * copy the options).
- *
- * @retval ::AMD_COMGR_STATUS_SUCCESS The function has
- * been executed successfully.
- *
- * @retval ::AMD_COMGR_STATUS_ERROR The options of @p action_info were not set
- * with @p amd_comgr_action_info_set_options.
- *
- * @retval ::AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT @p
- * action_info is an invalid action info object. @p size is NULL.
- *
- * @retval ::AMD_COMGR_STATUS_ERROR_OUT_OF_RESOURCES
- * Unable to update the data object as out of resources.
- *
- * @deprecated since 1.3
- * @see amd_comgr_action_info_get_option_list_count and
- * amd_comgr_action_info_get_option_list_item
- */
-AMD_COMGR_DEPRECATED("Will be removed in Comgr v3.0 (Rocm v6.0). Use "
-                     "amd_comgr_action_info_get_option_list_count() "
-                     "and amd_comgr_action_info_get_option_list_item() "
-                     "instead")
-amd_comgr_status_t AMD_COMGR_API
-amd_comgr_action_info_get_options(
-  amd_comgr_action_info_t action_info,
-  size_t *size,
-  char *options) AMD_COMGR_VERSION_1_8;
-
 /**
  * @brief Set the options array of an action info object.
  *
  * This overrides any option strings or arrays previously set by calls to this
- * function or @p amd_comgr_action_info_set_options.
+ * function.
  *
  * An @p action_info object which had its options set with this function can
  * only have its option inspected with @p
@@ -1385,7 +1304,7 @@ amd_comgr_action_info_get_option_list_count(
  *
  * @param[in] index The index of the option to be returned. The first option
  * index is 0. The order is the same as the options when they were added in @p
- * amd_comgr_action_info_set_options.
+ * amd_comgr_action_info_set_option_list.
  *
  * @param[in, out] size On entry, the size of @p option. On return, if @option
  * is NULL, set to the size of the Nth option string including the terminating
diff --git a/amd/comgr/src/amdcomgr.def b/amd/comgr/src/amdcomgr.def
@@ -24,8 +24,6 @@ amd_comgr_action_info_set_isa_name
 amd_comgr_action_info_get_isa_name
 amd_comgr_action_info_set_language
 amd_comgr_action_info_get_language
-amd_comgr_action_info_set_options
-amd_comgr_action_info_get_options
 amd_comgr_action_info_set_option_list
 amd_comgr_action_info_get_option_list_count
 amd_comgr_action_info_get_option_list_item
diff --git a/amd/comgr/src/comgr-device-libs.cpp b/amd/comgr/src/comgr-device-libs.cpp
@@ -131,7 +131,7 @@ amd_comgr_status_t addDeviceLibraries(DataAction *ActionInfo,
   bool CorrectlyRoundedSqrt = false, DazOpt = false, FiniteOnly = false,
        UnsafeMath = false, Wavefrontsize64 = false;
   unsigned CodeObjectVersion = 0;
-  for (auto &Option : ActionInfo->getOptions(true)) {
+  for (auto &Option : ActionInfo->getOptions()) {
     // Parse code_object_v<N>
     if (StringRef CoV = Option; CoV.consume_front("code_object_v")) {
       // String is invalid if:
diff --git a/amd/comgr/src/comgr.cpp b/amd/comgr/src/comgr.cpp
@@ -452,7 +452,7 @@ DataSet::~DataSet() {
 
 DataAction::DataAction()
     : IsaName(nullptr), Path(nullptr), Language(AMD_COMGR_LANGUAGE_NONE),
-      Logging(false), AreOptionsList(false) {}
+      Logging(false) {}
 
 DataAction::~DataAction() {
   free(IsaName);
@@ -467,22 +467,7 @@ amd_comgr_status_t DataAction::setActionPath(llvm::StringRef ActionPath) {
   return setCStr(this->Path, ActionPath);
 }
 
-amd_comgr_status_t DataAction::setOptionsFlat(StringRef Options) {
-  AreOptionsList = false;
-  FlatOptions = Options.str();
-  return AMD_COMGR_STATUS_SUCCESS;
-}
-
-amd_comgr_status_t DataAction::getOptionsFlat(StringRef &Options) {
-  if (AreOptionsList) {
-    return AMD_COMGR_STATUS_ERROR;
-  }
-  Options = StringRef(FlatOptions.c_str(), FlatOptions.size() + 1);
-  return AMD_COMGR_STATUS_SUCCESS;
-}
-
 amd_comgr_status_t DataAction::setOptionList(ArrayRef<const char *> Options) {
-  AreOptionsList = true;
   ListOptions.clear();
   for (auto &Option : Options) {
     ListOptions.push_back(Option);
@@ -491,18 +476,12 @@ amd_comgr_status_t DataAction::setOptionList(ArrayRef<const char *> Options) {
 }
 
 amd_comgr_status_t DataAction::getOptionListCount(size_t &Size) {
-  if (!AreOptionsList) {
-    return AMD_COMGR_STATUS_ERROR;
-  }
   Size = ListOptions.size();
   return AMD_COMGR_STATUS_SUCCESS;
 }
 
 amd_comgr_status_t DataAction::getOptionListItem(size_t Index,
                                                  StringRef &Option) {
-  if (!AreOptionsList) {
-    return AMD_COMGR_STATUS_ERROR;
-  }
   if (Index >= ListOptions.size()) {
     return AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT;
   }
@@ -511,25 +490,7 @@ amd_comgr_status_t DataAction::getOptionListItem(size_t Index,
   return AMD_COMGR_STATUS_SUCCESS;
 }
 
-ArrayRef<std::string> DataAction::getOptions(bool IsDeviceLibs) {
-  // In the legacy path the ListOptions is used as a buffer to split the
-  // options in. We have to do this lazily as the delimiter depends on
-  // IsDeviceLibs. We could avoid re-splitting in the case where the same call
-  // is repeated, but this path will be deprecated and removed anyway.
-  if (!AreOptionsList) {
-    ListOptions.clear();
-    StringRef OptionsRef(FlatOptions);
-    SmallVector<StringRef, 16> OptionRefs;
-    if (IsDeviceLibs) {
-      OptionsRef.split(OptionRefs, ',', -1, false);
-    } else {
-      OptionsRef.split(OptionRefs, ' ');
-    }
-    for (auto &Option : OptionRefs) {
-      ListOptions.push_back(std::string(Option));
-    }
-  }
-
+ArrayRef<std::string> DataAction::getOptions() {
   return ListOptions;
 }
 
@@ -1117,45 +1078,6 @@ amd_comgr_status_t AMD_COMGR_API
   return AMD_COMGR_STATUS_SUCCESS;
 }
 
-amd_comgr_status_t AMD_COMGR_API
-    // NOLINTNEXTLINE(readability-identifier-naming)
-    amd_comgr_action_info_set_options
-    //
-    (amd_comgr_action_info_t ActionInfo, const char *Options) {
-  DataAction *ActionP = DataAction::convert(ActionInfo);
-
-  if (!ActionP) {
-    return AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT;
-  }
-
-  return ActionP->setOptionsFlat(Options);
-}
-
-amd_comgr_status_t AMD_COMGR_API
-    // NOLINTNEXTLINE(readability-identifier-naming)
-    amd_comgr_action_info_get_options
-    //
-    (amd_comgr_action_info_t ActionInfo, size_t *Size, char *Options) {
-  DataAction *ActionP = DataAction::convert(ActionInfo);
-
-  if (!ActionP || !Size) {
-    return AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT;
-  }
-
-  StringRef ActionOptions;
-  if (auto Status = ActionP->getOptionsFlat(ActionOptions)) {
-    return Status;
-  }
-
-  if (Options) {
-    memcpy(Options, ActionOptions.data(), *Size);
-  } else {
-    *Size = ActionOptions.size();
-  }
-
-  return AMD_COMGR_STATUS_SUCCESS;
-}
-
 amd_comgr_status_t AMD_COMGR_API
     // NOLINTNEXTLINE(readability-identifier-naming)
     amd_comgr_action_info_set_option_list
@@ -1427,8 +1349,7 @@ amd_comgr_status_t AMD_COMGR_API
         << "\t  ActionKind: " << getActionKindName(ActionKind) << '\n'
         << "\t     IsaName: " << ActionInfoP->IsaName << '\n'
         << "\t     Options:";
-      for (auto &Option : ActionInfoP->getOptions(
-          ActionKind == AMD_COMGR_ACTION_ADD_DEVICE_LIBRARIES)) {
+      for (auto &Option : ActionInfoP->getOptions()) {
         *LogP << ' ';
         printQuotedOption(*LogP, Option);
       }
diff --git a/amd/comgr/src/comgr.h b/amd/comgr/src/comgr.h
@@ -210,12 +210,6 @@ struct DataAction {
   amd_comgr_status_t setIsaName(llvm::StringRef IsaName);
   amd_comgr_status_t setActionPath(llvm::StringRef ActionPath);
 
-  // Set the options to be the legacy "flat" string.
-  amd_comgr_status_t setOptionsFlat(llvm::StringRef Options);
-  // If the options were set via setOptionsFlag, return a reference to the
-  // string (including the null terminator).
-  amd_comgr_status_t getOptionsFlat(llvm::StringRef &Options);
-
   // Set the options to be the new list.
   amd_comgr_status_t setOptionList(llvm::ArrayRef<const char *> Options);
   // If the options were set via setOptionList, return the length of the list.
@@ -224,11 +218,9 @@ struct DataAction {
   // string at Index in the list (including the null terminator).
   amd_comgr_status_t getOptionListItem(size_t Index, llvm::StringRef &Option);
 
-  // Return a normalized array of options, possibly splitting a flat options
-  // string. If splitting, ' ' is used as a delimiter if IsDeviceLibs is false,
-  // otherwise ',' is used. The returned array reference is only valid as long
-  // as no other option APIs are called.
-  llvm::ArrayRef<std::string> getOptions(bool IsDeviceLibs = false);
+  // Return an array of options. The returned array reference is only valid as
+  // long as no other option APIs are called.
+  llvm::ArrayRef<std::string> getOptions();
 
   amd_comgr_status_t setBundleEntryIDs(llvm::ArrayRef<const char *> EntryIDs);
   llvm::ArrayRef<std::string> getBundleEntryIDs();
@@ -242,8 +234,6 @@ struct DataAction {
   std::vector<std::string> BundleEntryIDs;
 
 private:
-  bool AreOptionsList;
-  std::string FlatOptions;
   std::vector<std::string> ListOptions;
 };
 
diff --git a/amd/comgr/src/exportmap.in b/amd/comgr/src/exportmap.in
@@ -5,12 +5,10 @@ global: amd_comgr_action_data_count;
         amd_comgr_action_info_get_logging;
         amd_comgr_action_info_get_option_list_count;
         amd_comgr_action_info_get_option_list_item;
-        amd_comgr_action_info_get_options;
         amd_comgr_action_info_get_working_directory_path;
         amd_comgr_action_info_set_language;
         amd_comgr_action_info_set_logging;
         amd_comgr_action_info_set_option_list;
-        amd_comgr_action_info_set_options;
         amd_comgr_action_info_set_working_directory_path;
         amd_comgr_create_action_info;
         amd_comgr_create_data_set;
diff --git a/amd/comgr/test/options_test.c b/amd/comgr/test/options_test.c
