[Comgr][v3.0] Remove deprecated HIP compilation workaround

 - AMD_COMGR_COMPILE_SOURCE_TO_FATBIN

Change-Id: I257207c1df624b2e068538a3f40a730c98138537

Author: lamb-j

amd/comgr/docs/ReleaseNotes.md

@@ -160,6 +160,9 @@ Deprecated Comgr Actions and Data Types
 
 Removed Comgr Actions and Data Types
 ------------------------------------
+- (Action) AMD\_COMGR\_ACTION\_COMPILE\_SOURCE\_TO\_FATBIN
+  - This workaround has been removed in favor of
+  \*\_COMPILE\_SOURCE\_(WITH\_DEVICE\_LIBS\_)TO\_BC
 
 Comgr Testing, Debugging, and Logging Updates
 ---------------------------------------------

amd/comgr/include/amd_comgr.h.in

@@ -1760,28 +1760,6 @@ typedef enum amd_comgr_action_kind_s {
    * if isa name is not set in @p info
    */
   AMD_COMGR_ACTION_DISASSEMBLE_BYTES_TO_SOURCE = 0xD,
-  /**
-   * Compile each source data object in @p input in order. For each
-   * successful compilation add a fat binary to @p result. Resolve
-   * any include source names using the names of include data objects
-   * in @p input. Resolve any include relative path names using the
-   * working directory path in @p info. Produce fat binary for isa name in @p
-   * info. Compile the source for the language in @p info.
-   *
-   * Return @p AMD_COMGR_STATUS_ERROR if any compilation
-   * fails.
-   *
-   * Return @p AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT
-   * if isa name or language is not set in @p info.
-   *
-   * @deprecated since 2.5
-   * @see in-process compilation via AMD_COMGR_ACTION_COMPILE_SOURCE_TO_BC, etc.
-   * insteaad
-   */
-  AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN
-  AMD_COMGR_DEPRECATED("Will be removed in Comgr v3.0 (Rocm v6.0). Use "
-                       "AMD_COMGR_ACTION_COMPILE_SOURCE_TO_BC, etc. "
-                       "instead") = 0xE,
   /**
    * Compile each source data object in @p input in order. For each
    * successful compilation add a bc data object to @p result. Resolve
@@ -1798,7 +1776,7 @@ typedef enum amd_comgr_action_kind_s {
    * Return @p AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT
    * if isa name or language is not set in @p info.
    */
-  AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC = 0xF,
+  AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC = 0xE,
   /**
    * Compile a single source data object in @p input in order. For each
    * successful compilation add a relocatable data object to @p result.
@@ -1815,7 +1793,7 @@ typedef enum amd_comgr_action_kind_s {
    * Return @p AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT
    * if isa name or language is not set in @p info.
    */
-  AMD_COMGR_ACTION_COMPILE_SOURCE_TO_RELOCATABLE = 0x10,
+  AMD_COMGR_ACTION_COMPILE_SOURCE_TO_RELOCATABLE = 0xF,
   /**
    * Compile each source data object in @p input and create a single executabele
    * in @p result. Resolve any include source names using the names of include
@@ -1831,7 +1809,7 @@ typedef enum amd_comgr_action_kind_s {
    * Return @p AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT
    * if isa name or language is not set in @p info.
    */
-  AMD_COMGR_ACTION_COMPILE_SOURCE_TO_EXECUTABLE = 0x11,
+  AMD_COMGR_ACTION_COMPILE_SOURCE_TO_EXECUTABLE = 0x10,
 
    /**
    * Unbundle each source data object in @p input. These objects can be
@@ -1845,7 +1823,7 @@ typedef enum amd_comgr_action_kind_s {
    * Return @p AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT
    * if isa name or language is not set in @p info.
    */
-  AMD_COMGR_ACTION_UNBUNDLE = 0x12,
+  AMD_COMGR_ACTION_UNBUNDLE = 0x11,
 
   /**
    * Marker for last valid action kind.

amd/comgr/src/comgr-compiler.cpp

@@ -837,44 +837,6 @@ amd_comgr_status_t AMDGPUCompiler::removeTmpDirs() {
 #endif
 }
 
-amd_comgr_status_t AMDGPUCompiler::executeOutOfProcessHIPCompilation(
-    llvm::ArrayRef<const char *> Args) {
-  std::string Exec = (Twine(env::getHIPPath()) + "/bin/hipcc").str();
-  std::vector<StringRef> ArgsV;
-  ArgsV.push_back(Exec);
-  for (unsigned I = 0, E = Args.size(); I != E; ++I) {
-    if (strcmp(Args[I], "-hip-path") == 0) {
-      ++I;
-      if (I == E) {
-        LogS << "Error: -hip-path option misses argument.\n";
-        return AMD_COMGR_STATUS_ERROR;
-      }
-      Exec = (Twine(Args[I]) + "/bin/hipcc").str();
-      ArgsV[0] = Exec;
-
-    } else {
-      ArgsV.push_back(Args[I]);
-    }
-  }
-
-  ArgsV.push_back("--genco");
-
-  if (env::shouldEmitVerboseLogs()) {
-    LogS << "\t    hipcc Command: ";
-    for (auto A : ArgsV)
-      LogS << A << " ";
-    LogS << "\n";
-  }
-
-  llvm::ArrayRef<std::optional<StringRef>> Redirects;
-  std::string ErrMsg;
-  int RC = sys::ExecuteAndWait(Exec, ArgsV,
-                               /*env=*/std::nullopt, Redirects, /*secondsToWait=*/0,
-                               /*memoryLimit=*/0, &ErrMsg);
-  LogS << ErrMsg;
-  return RC ? AMD_COMGR_STATUS_ERROR : AMD_COMGR_STATUS_SUCCESS;
-}
-
 amd_comgr_status_t AMDGPUCompiler::processFile(const char *InputFilePath,
                                                const char *OutputFilePath) {
   SmallVector<const char *, 128> Argv = Args;
@@ -901,11 +863,6 @@ amd_comgr_status_t AMDGPUCompiler::processFile(const char *InputFilePath,
   Argv.push_back("-o");
   Argv.push_back(OutputFilePath);
 
-  // For HIP OOP compilation, we launch a process.
-  if (CompileOOP && getLanguage() == AMD_COMGR_LANGUAGE_HIP) {
-    return executeOutOfProcessHIPCompilation(Argv);
-  }
-
   return executeInProcessDriver(Argv);
 }
 
@@ -1243,23 +1200,6 @@ amd_comgr_status_t AMDGPUCompiler::compileToRelocatable() {
   return processFiles(AMD_COMGR_DATA_KIND_RELOCATABLE, ".o");
 }
 
-amd_comgr_status_t AMDGPUCompiler::compileToFatBin() {
-  if (auto Status = createTmpDirs()) {
-    return Status;
-  }
-
-  if (ActionInfo->Language != AMD_COMGR_LANGUAGE_HIP) {
-    return AMD_COMGR_STATUS_ERROR_INVALID_ARGUMENT;
-  }
-
-  // This is a workaround to support HIP OOP Fatbin Compilation
-  CompileOOP = true;
-  auto Status = processFiles(AMD_COMGR_DATA_KIND_FATBIN, ".fatbin");
-  CompileOOP = false;
-
-  return Status;
-}
-
 amd_comgr_status_t AMDGPUCompiler::unbundle() {
   if (auto Status = createTmpDirs()) {
     return Status;

amd/comgr/src/comgr-compiler.h

@@ -94,8 +94,6 @@ class AMDGPUCompiler {
   std::string HIPIncludePath;
   std::string ClangIncludePath;
   std::string ClangIncludePath2;
-  /// Perform out-of-process compilation.
-  bool CompileOOP = false;
   /// Precompiled header file paths.
   llvm::SmallVector<llvm::SmallString<128>, 2> PrecompiledHeaders;
   /// Arguments common to all driver invocations in the current action.
@@ -123,8 +121,6 @@ class AMDGPUCompiler {
                                               bool CompilingSrc);
   amd_comgr_status_t addCompilationFlags();
   amd_comgr_status_t addDeviceLibraries();
-  amd_comgr_status_t
-  executeOutOfProcessHIPCompilation(llvm::ArrayRef<const char *> Args);
 
   amd_comgr_status_t executeInProcessDriver(llvm::ArrayRef<const char *> Args);
 
@@ -143,7 +139,6 @@ class AMDGPUCompiler {
   amd_comgr_status_t assembleToRelocatable();
   amd_comgr_status_t linkToRelocatable();
   amd_comgr_status_t linkToExecutable();
-  amd_comgr_status_t compileToFatBin();
   amd_comgr_status_t compileToExecutable();
 
   amd_comgr_language_t getLanguage() const { return ActionInfo->Language; }

amd/comgr/src/comgr.cpp

@@ -179,8 +179,6 @@ dispatchCompilerAction(amd_comgr_action_kind_t ActionKind,
     return Compiler.linkToRelocatable();
   case AMD_COMGR_ACTION_LINK_RELOCATABLE_TO_EXECUTABLE:
     return Compiler.linkToExecutable();
-  case AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN:
-    return Compiler.compileToFatBin();
   case AMD_COMGR_ACTION_COMPILE_SOURCE_TO_RELOCATABLE:
     return Compiler.compileToRelocatable();
   case AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC:
@@ -243,8 +241,6 @@ StringRef getActionKindName(amd_comgr_action_kind_t ActionKind) {
     return "AMD_COMGR_ACTION_DISASSEMBLE_BYTES_TO_SOURCE";
   case AMD_COMGR_ACTION_COMPILE_SOURCE_TO_RELOCATABLE:
     return "AMD_COMGR_ACTION_COMPILE_SOURCE_TO_RELOCATABLE";
-  case AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN:
-    return "AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN";
   case AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC:
     return "AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC";
   case AMD_COMGR_ACTION_COMPILE_SOURCE_TO_EXECUTABLE:
@@ -1381,7 +1377,6 @@ amd_comgr_status_t AMD_COMGR_API
     case AMD_COMGR_ACTION_LINK_RELOCATABLE_TO_RELOCATABLE:
     case AMD_COMGR_ACTION_LINK_RELOCATABLE_TO_EXECUTABLE:
     case AMD_COMGR_ACTION_COMPILE_SOURCE_TO_RELOCATABLE:
-    case AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN:
     case AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC:
     case AMD_COMGR_ACTION_COMPILE_SOURCE_TO_EXECUTABLE:
       ActionStatus = dispatchCompilerAction(ActionKind, ActionInfoP, InputSetP,

amd/comgr/test/CMakeLists.txt

@@ -242,7 +242,6 @@ if (DEFINED HIP_COMPILER AND "${HIP_COMPILER}" STREQUAL "clang")
   add_test_archive(cube_archive cube source/cube.bc source/cube.a)
 
   add_comgr_test(compile_hip_test c)
-  add_comgr_test(compile_hip_test_in_process c)
   add_comgr_test(unbundle_hip_test c)
   add_comgr_test(compile_hip_to_relocatable c)
   add_comgr_test(mangled_names_hip_test c)

amd/comgr/test/compile_hip_test.c

@@ -39,65 +39,83 @@
 #include <stdlib.h>
 #include <string.h>
 
-int main(int argc, char *argv[]) {
-  char *BufSource1;
-  size_t SizeSource1;
-  amd_comgr_data_t DataSource1;
-  amd_comgr_data_set_t DataSetIn, DataSetFatBin;
-  amd_comgr_action_info_t DataAction;
+int main(int Argc, char *Argv[]) {
+  char *BufSource;
+  size_t SizeSource;
+  amd_comgr_data_t DataSrc;
+  amd_comgr_data_set_t DataSetSrc, DataSetBc, DataSetLinkedBc,
+      DataSetAsm, DataSetReloc, DataSetExec;
+  amd_comgr_action_info_t ActionInfo;
   amd_comgr_status_t Status;
-  size_t Count;
-  const char *Options[] = {"--amdgpu-target=gfx900"};
-  size_t OptionsCount = sizeof(Options) / sizeof(Options[0]);
 
-  SizeSource1 = setBuf(TEST_OBJ_DIR "/source1.hip", &BufSource1);
+  SizeSource = setBuf(TEST_OBJ_DIR "/source2.hip", &BufSource);
 
-  Status = amd_comgr_create_data_set(&DataSetIn);
-  checkError(Status, "amd_comgr_create_data_set");
-
-  Status = amd_comgr_create_data(AMD_COMGR_DATA_KIND_SOURCE, &DataSource1);
+  Status = amd_comgr_create_data(AMD_COMGR_DATA_KIND_SOURCE, &DataSrc);
   checkError(Status, "amd_comgr_create_data");
-  Status = amd_comgr_set_data(DataSource1, SizeSource1, BufSource1);
+  Status = amd_comgr_set_data(DataSrc, SizeSource, BufSource);
   checkError(Status, "amd_comgr_set_data");
-  Status = amd_comgr_set_data_name(DataSource1, "source1.hip");
+  Status = amd_comgr_set_data_name(DataSrc, "source2.hip");
   checkError(Status, "amd_comgr_set_data_name");
-  Status = amd_comgr_data_set_add(DataSetIn, DataSource1);
+  Status = amd_comgr_create_data_set(&DataSetSrc);
+  checkError(Status, "amd_comgr_create_data_set");
+  Status = amd_comgr_data_set_add(DataSetSrc, DataSrc);
   checkError(Status, "amd_comgr_data_set_add");
 
-  Status = amd_comgr_create_action_info(&DataAction);
+  Status = amd_comgr_create_action_info(&ActionInfo);
   checkError(Status, "amd_comgr_create_action_info");
   Status =
-      amd_comgr_action_info_set_language(DataAction, AMD_COMGR_LANGUAGE_HIP);
+      amd_comgr_action_info_set_language(ActionInfo, AMD_COMGR_LANGUAGE_HIP);
   checkError(Status, "amd_comgr_action_info_set_language");
-  Status =
-      amd_comgr_action_info_set_option_list(DataAction, Options, OptionsCount);
-  checkError(Status, "amd_comgr_action_info_set_option_list");
+  Status = amd_comgr_action_info_set_isa_name(ActionInfo,
+                                              "amdgcn-amd-amdhsa--gfx906");
+  checkError(Status, "amd_comgr_action_info_set_isa_name");
 
-  Status = amd_comgr_create_data_set(&DataSetFatBin);
+  Status = amd_comgr_create_data_set(&DataSetBc);
   checkError(Status, "amd_comgr_create_data_set");
+  Status = amd_comgr_do_action(AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC,
+                               ActionInfo, DataSetSrc, DataSetBc);
+  checkError(Status, "amd_comgr_do_action");
 
-  Status = amd_comgr_do_action(AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN,
-                               DataAction, DataSetIn, DataSetFatBin);
+  Status = amd_comgr_create_data_set(&DataSetLinkedBc);
+  checkError(Status, "amd_comgr_create_data_set");
+  Status = amd_comgr_do_action(AMD_COMGR_ACTION_LINK_BC_TO_BC, ActionInfo,
+                               DataSetBc, DataSetLinkedBc);
   checkError(Status, "amd_comgr_do_action");
 
-  Status = amd_comgr_action_data_count(DataSetFatBin,
-                                       AMD_COMGR_DATA_KIND_FATBIN, &Count);
-  checkError(Status, "amd_comgr_action_data_count");
+  Status = amd_comgr_create_data_set(&DataSetAsm);
+  checkError(Status, "amd_comgr_create_data_set");
+  Status = amd_comgr_do_action(AMD_COMGR_ACTION_CODEGEN_BC_TO_ASSEMBLY,
+                               ActionInfo, DataSetLinkedBc, DataSetAsm);
+  checkError(Status, "amd_comgr_do_action");
 
-  if (Count != 1) {
-    printf("AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN Failed: "
-           "produced %zu fab binaries (expected 1)\n",
-           Count);
-    exit(1);
-  }
+  Status = amd_comgr_create_data_set(&DataSetReloc);
+  checkError(Status, "amd_comgr_create_data_set");
+  Status = amd_comgr_do_action(AMD_COMGR_ACTION_CODEGEN_BC_TO_RELOCATABLE,
+                               ActionInfo, DataSetLinkedBc, DataSetReloc);
+  checkError(Status, "amd_comgr_do_action");
 
-  Status = amd_comgr_release_data(DataSource1);
-  checkError(Status, "amd_comgr_release_data");
-  Status = amd_comgr_destroy_data_set(DataSetIn);
+  Status = amd_comgr_create_data_set(&DataSetExec);
+  checkError(Status, "amd_comgr_create_data_set");
+  Status = amd_comgr_do_action(AMD_COMGR_ACTION_LINK_RELOCATABLE_TO_EXECUTABLE,
+                               ActionInfo, DataSetReloc, DataSetExec);
+  checkError(Status, "amd_comgr_do_action");
+
+  Status = amd_comgr_destroy_action_info(ActionInfo);
+  checkError(Status, "amd_comgr_destroy_action_info");
+  Status = amd_comgr_destroy_data_set(DataSetSrc);
   checkError(Status, "amd_comgr_destroy_data_set");
-  Status = amd_comgr_destroy_data_set(DataSetFatBin);
+  Status = amd_comgr_destroy_data_set(DataSetBc);
   checkError(Status, "amd_comgr_destroy_data_set");
-  Status = amd_comgr_destroy_action_info(DataAction);
-  checkError(Status, "amd_comgr_destroy_action_info");
-  free(BufSource1);
+  Status = amd_comgr_destroy_data_set(DataSetLinkedBc);
+  checkError(Status, "amd_comgr_destroy_data_set");
+  Status = amd_comgr_destroy_data_set(DataSetAsm);
+  checkError(Status, "amd_comgr_destroy_data_set");
+  Status = amd_comgr_destroy_data_set(DataSetReloc);
+  checkError(Status, "amd_comgr_destroy_data_set");
+  Status = amd_comgr_destroy_data_set(DataSetExec);
+  checkError(Status, "amd_comgr_destroy_data_set");
+  Status = amd_comgr_release_data(DataSrc);
+  checkError(Status, "amd_comgr_release_data");
+
+  free(BufSource);
 }