yaxunl updated this revision to Diff 462673.
yaxunl added a comment.

I just found clang-offload-bundler reports an error when trying to unbundle an 
archive but the input file is not an archive.

This update let clang-offload-bundler to extract empty archives when the input 
file is not an archive and -allow-missing-bundles is specified.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133705/new/

https://reviews.llvm.org/D133705

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/OffloadBundler.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/clang-offload-bundler.c
  clang/test/Driver/hip-link-bundle-archive.hip

Index: clang/test/Driver/hip-link-bundle-archive.hip
===================================================================
--- clang/test/Driver/hip-link-bundle-archive.hip
+++ clang/test/Driver/hip-link-bundle-archive.hip
@@ -8,7 +8,23 @@
 // RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
 // RUN:   --target=x86_64-unknown-linux-gnu \
 // RUN:   -nogpulib %s -fgpu-rdc -L%t -lhipBundled \
-// RUN:   2>&1 | FileCheck -check-prefix=GNU %s
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU1,GNU-L %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   -nogpulib %s -fgpu-rdc -L%t -l:libhipBundled.a \
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU1,GNU-LA %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   -nogpulib %s -fgpu-rdc %t/libhipBundled.a \
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU1,GNU-A %s
+
+// RUN: touch %t/libhipBundled.a.5.2
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-unknown-linux-gnu \
+// RUN:   -nogpulib %s -fgpu-rdc %t/libhipBundled.a.5.2 \
+// RUN:   2>&1 | FileCheck -check-prefixes=GNU,GNU2,GNU-A %s
 
 // RUN: touch %t/hipBundled2.lib
 // RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
@@ -16,14 +32,27 @@
 // RUN:   -nogpulib %s -fgpu-rdc -L%t -lhipBundled2 \
 // RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
 
-// GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libhipBundled.a" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-pc-windows-msvc \
+// RUN:   -nogpulib %s -fgpu-rdc -L%t -l:hipBundled2.lib \
+// RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN:   --target=x86_64-pc-windows-msvc \
+// RUN:   -nogpulib %s -fgpu-rdc %t/hipBundled2.lib \
+// RUN:   2>&1 | FileCheck -check-prefix=MSVC %s
+
+// GNU1: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}[[LIB:libhipBundled\.a]]" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
+// GNU2: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}[[LIB:libhipBundled\.a\.5\.2]]" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
 // GNU: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx1030" {{.*}} "[[A1030]]"
-// GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libhipBundled.a" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
+// GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}[[LIB]]" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
 // GNU: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx906" {{.*}} "[[A906]]"
-// GNU: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-lhipBundled"
+// GNU-L: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-lhipBundled"
+// GNU-LA: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-l:libhipBundled.a"
+// GNU-A: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" "{{.*}}[[LIB]]"
 
 // MSVC: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}hipBundled2.lib" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
 // MSVC: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx1030" {{.*}} "[[A1030]]"
 // MSVC: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}hipBundled2.lib" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
 // MSVC: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx906" {{.*}} "[[A906]]"
-// MSVC: "{{.*}}link{{.*}}" {{.*}}"-out:a.exe" {{.*}}"hipBundled2.lib"
+// MSVC: "{{.*}}link{{.*}}" {{.*}}"-out:a.exe" {{.*}}hipBundled2.lib"
Index: clang/test/Driver/clang-offload-bundler.c
===================================================================
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -407,6 +407,19 @@
 // HIP-AR-906-DAG: hip_bundle1-hip-amdgcn-amd-amdhsa--gfx906
 // HIP-AR-906-DAG: hip_bundle2-hip-amdgcn-amd-amdhsa--gfx906
 
+// Check clang-offload-bundler extracts empty archives if the input file
+// is not an archive when --allow-missing-bundles is specified, otherwise
+// report an error.
+//
+// RUN: echo 'This is not an archive file.' > %t.non-archive
+// RUN: clang-offload-bundler -unbundle -type=a -targets=hip-amdgcn-amd-amdhsa--gfx900 \
+// RUN:   -output=%t.res.a -input=%t.non-archive --allow-missing-bundles
+// RUN: llvm-ar t %t.res.a >%t.res.list 2>&1
+// RUN: diff %t.res.list %t.empty
+// RUN: not clang-offload-bundler -unbundle -type=a -targets=hip-amdgcn-amd-amdhsa--gfx900 \
+// RUN:   -output=%t.res.a -input=%t.non-archive 2>&1 | FileCheck %s -check-prefix=INVARCHIVE
+// INVARCHIVE: error: file too small to be an archive
+
 //
 // Check bundling without host target is allowed for HIP.
 //
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===================================================================
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1812,85 +1812,94 @@
     return false;
 
   bool FoundAOB = false;
-  SmallVector<std::string, 2> AOBFileNames;
   std::string ArchiveOfBundles;
-  for (auto LPath : LibraryPaths) {
-    ArchiveOfBundles.clear();
-
-    llvm::Triple Triple(D.getTargetTriple());
-    bool IsMSVC = Triple.isWindowsMSVCEnvironment();
-    for (auto Prefix : {"/libdevice/", "/"}) {
-      if (IsMSVC)
-        AOBFileNames.push_back(Twine(LPath + Prefix + Lib + ".lib").str());
-      AOBFileNames.push_back(Twine(LPath + Prefix + "lib" + Lib + ".a").str());
-    }
 
-    for (auto AOB : AOBFileNames) {
-      if (llvm::sys::fs::exists(AOB)) {
-        ArchiveOfBundles = AOB;
-        FoundAOB = true;
-        break;
+  llvm::Triple Triple(D.getTargetTriple());
+  bool IsMSVC = Triple.isWindowsMSVCEnvironment();
+  auto Ext = IsMSVC ? ".lib" : ".a";
+  if (!Lib.startswith(":") && llvm::sys::fs::exists(Lib)) {
+    ArchiveOfBundles = Lib;
+    FoundAOB = true;
+  } else {
+    for (auto LPath : LibraryPaths) {
+      ArchiveOfBundles.clear();
+      SmallVector<std::string, 2> AOBFileNames;
+      auto LibFile =
+          (Lib.startswith(":") ? Lib.drop_front()
+                               : IsMSVC ? Lib + Ext : "lib" + Lib + Ext)
+              .str();
+      for (auto Prefix : {"/libdevice/", "/"}) {
+        auto AOB = Twine(LPath + Prefix + LibFile).str();
+        if (llvm::sys::fs::exists(AOB)) {
+          ArchiveOfBundles = AOB;
+          FoundAOB = true;
+          break;
+        }
       }
+      if (FoundAOB)
+        break;
     }
+  }
 
-    if (!FoundAOB)
-      continue;
-
-    StringRef Prefix = isBitCodeSDL ? "libbc-" : "lib";
-    std::string OutputLib = D.GetTemporaryPath(
-        Twine(Prefix + Lib + "-" + Arch + "-" + Target).str(), "a");
-
-    C.addTempFile(C.getArgs().MakeArgString(OutputLib));
+  if (!FoundAOB)
+    return false;
 
-    ArgStringList CmdArgs;
-    SmallString<128> DeviceTriple;
-    DeviceTriple += Action::GetOffloadKindName(JA.getOffloadingDeviceKind());
-    DeviceTriple += '-';
-    std::string NormalizedTriple = T.getToolChain().getTriple().normalize();
-    DeviceTriple += NormalizedTriple;
+  StringRef Prefix = isBitCodeSDL ? "libbc-" : "lib";
+  std::string OutputLib =
+      D.GetTemporaryPath(Twine(Prefix + llvm::sys::path::filename(Lib) + "-" +
+                               Arch + "-" + Target)
+                             .str(),
+                         "a");
+
+  C.addTempFile(C.getArgs().MakeArgString(OutputLib));
+
+  ArgStringList CmdArgs;
+  SmallString<128> DeviceTriple;
+  DeviceTriple += Action::GetOffloadKindName(JA.getOffloadingDeviceKind());
+  DeviceTriple += '-';
+  std::string NormalizedTriple = T.getToolChain().getTriple().normalize();
+  DeviceTriple += NormalizedTriple;
     if (!Target.empty()) {
-      DeviceTriple += '-';
+    DeviceTriple += '-';
       DeviceTriple += Target;
-    }
+  }
 
-    std::string UnbundleArg("-unbundle");
-    std::string TypeArg("-type=a");
-    std::string InputArg("-input=" + ArchiveOfBundles);
-    std::string OffloadArg("-targets=" + std::string(DeviceTriple));
-    std::string OutputArg("-output=" + OutputLib);
-
-    const char *UBProgram = DriverArgs.MakeArgString(
-        T.getToolChain().GetProgramPath("clang-offload-bundler"));
-
-    ArgStringList UBArgs;
-    UBArgs.push_back(C.getArgs().MakeArgString(UnbundleArg));
-    UBArgs.push_back(C.getArgs().MakeArgString(TypeArg));
-    UBArgs.push_back(C.getArgs().MakeArgString(InputArg));
-    UBArgs.push_back(C.getArgs().MakeArgString(OffloadArg));
-    UBArgs.push_back(C.getArgs().MakeArgString(OutputArg));
-
-    // Add this flag to not exit from clang-offload-bundler if no compatible
-    // code object is found in heterogenous archive library.
-    std::string AdditionalArgs("-allow-missing-bundles");
-    UBArgs.push_back(C.getArgs().MakeArgString(AdditionalArgs));
-
-    // Add this flag to treat hip and hipv4 offload kinds as compatible with
-    // openmp offload kind while extracting code objects from a heterogenous
-    // archive library. Vice versa is also considered compatible.
-    std::string HipCompatibleArgs("-hip-openmp-compatible");
-    UBArgs.push_back(C.getArgs().MakeArgString(HipCompatibleArgs));
-
-    C.addCommand(std::make_unique<Command>(
-        JA, T, ResponseFileSupport::AtFileCurCP(), UBProgram, UBArgs, Inputs,
-        InputInfo(&JA, C.getArgs().MakeArgString(OutputLib))));
-    if (postClangLink)
-      CC1Args.push_back("-mlink-builtin-bitcode");
+  std::string UnbundleArg("-unbundle");
+  std::string TypeArg("-type=a");
+  std::string InputArg("-input=" + ArchiveOfBundles);
+  std::string OffloadArg("-targets=" + std::string(DeviceTriple));
+  std::string OutputArg("-output=" + OutputLib);
 
-    CC1Args.push_back(DriverArgs.MakeArgString(OutputLib));
-    break;
-  }
+  const char *UBProgram = DriverArgs.MakeArgString(
+      T.getToolChain().GetProgramPath("clang-offload-bundler"));
+
+  ArgStringList UBArgs;
+  UBArgs.push_back(C.getArgs().MakeArgString(UnbundleArg));
+  UBArgs.push_back(C.getArgs().MakeArgString(TypeArg));
+  UBArgs.push_back(C.getArgs().MakeArgString(InputArg));
+  UBArgs.push_back(C.getArgs().MakeArgString(OffloadArg));
+  UBArgs.push_back(C.getArgs().MakeArgString(OutputArg));
+
+  // Add this flag to not exit from clang-offload-bundler if no compatible
+  // code object is found in heterogenous archive library.
+  std::string AdditionalArgs("-allow-missing-bundles");
+  UBArgs.push_back(C.getArgs().MakeArgString(AdditionalArgs));
+
+  // Add this flag to treat hip and hipv4 offload kinds as compatible with
+  // openmp offload kind while extracting code objects from a heterogenous
+  // archive library. Vice versa is also considered compatible.
+  std::string HipCompatibleArgs("-hip-openmp-compatible");
+  UBArgs.push_back(C.getArgs().MakeArgString(HipCompatibleArgs));
 
-  return FoundAOB;
+  C.addCommand(std::make_unique<Command>(
+      JA, T, ResponseFileSupport::AtFileCurCP(), UBProgram, UBArgs, Inputs,
+      InputInfo(&JA, C.getArgs().MakeArgString(OutputLib))));
+  if (postClangLink)
+    CC1Args.push_back("-mlink-builtin-bitcode");
+
+  CC1Args.push_back(DriverArgs.MakeArgString(OutputLib));
+
+  return true;
 }
 
 // Wrapper function used by driver for adding SDLs during link phase.
@@ -1980,6 +1989,22 @@
     }
   }
 
+  for (auto Input : DriverArgs.getAllArgValues(options::OPT_INPUT)) {
+    auto FileName = StringRef(Input);
+    // Clang treats any unknown file types as archives and passes them to the
+    // linker. Files with extension 'lib' are classified as TY_Object by clang
+    // but they are usually archives. It is OK if the file is not really an
+    // archive since in that case the unbundler just extracts an empty bitcode
+    // to link with.
+    const StringRef LibFileExt = ".lib";
+    if (!llvm::sys::path::has_extension(FileName) ||
+        types::lookupTypeForExtension(
+            llvm::sys::path::extension(FileName).drop_front()) ==
+            types::TY_INVALID ||
+        llvm::sys::path::extension(FileName) == LibFileExt)
+      SDLNames.insert(Input);
+  }
+
   // The search stops as soon as an SDL file is found. The driver then provides
   // the full filename of the SDL to the llvm-link or clang-nvlink-wrapper
   // command. If no SDL is found after searching each LINKPATH with
Index: clang/lib/Driver/OffloadBundler.cpp
===================================================================
--- clang/lib/Driver/OffloadBundler.cpp
+++ clang/lib/Driver/OffloadBundler.cpp
@@ -1139,8 +1139,24 @@
   ArchiveBuffers.push_back(std::move(*BufOrErr));
   Expected<std::unique_ptr<llvm::object::Archive>> LibOrErr =
       Archive::create(ArchiveBuffers.back()->getMemBufferRef());
-  if (!LibOrErr)
-    return LibOrErr.takeError();
+  // If it is not an archive, create an empty archive for each target if missing
+  // bundles are allowed.
+  if (Error E = LibOrErr.takeError()) {
+    if (!BundlerConfig.AllowMissingBundles)
+      return E;
+    // Silence the error.
+    (void)toString(std::move(E));
+    for (auto &Target : BundlerConfig.TargetNames) {
+      StringRef FileName = TargetOutputFileNameMap[Target];
+      std::vector<llvm::NewArchiveMember> EmptyArchive;
+      EmptyArchive.clear();
+      if (Error WriteErr = writeArchive(FileName, EmptyArchive, true,
+                                        getDefaultArchiveKindForHost(), true,
+                                        false, nullptr))
+        return WriteErr;
+    }
+    return Error::success();
+  }
 
   auto Archive = std::move(*LibOrErr);
 
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2903,12 +2903,16 @@
         std::string FileName = IA->getInputArg().getAsString(Args);
         // Check if the type of the file is the same as the action. Do not
         // unbundle it if it is not. Do not unbundle .so files, for example,
-        // which are not object files.
+        // which are not object files. Files with extension ".lib" is classified
+        // as TY_Object but they are actually archives, therefore should not be
+        // unbundled here as objects. They will be handled at other places.
+        const StringRef LibFileExt = ".lib";
         if (IA->getType() == types::TY_Object &&
             (!llvm::sys::path::has_extension(FileName) ||
              types::lookupTypeForExtension(
                  llvm::sys::path::extension(FileName).drop_front()) !=
-                 types::TY_Object))
+                 types::TY_Object ||
+             llvm::sys::path::extension(FileName) == LibFileExt))
           return ABRT_Inactive;
 
         for (auto Arch : GpuArchList) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to