jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield, tra, 
yaxunl.
Herald added a subscriber: kosarev.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: openmp-commits, cfe-commits, sstefan1.
Herald added projects: clang, OpenMP.
Currently, we pull in every single static archive member as long as we
have an offloading architecture that requires it. This goes against the
standard sematnics of static libraries that only pull in symbols that
define currently undefined symbols. In order to support this we roll
some custom symbol resolution logic to check if a static library is
needed. Because of offloading semantics, this requires an extra check
for externally visibile symbols. E.g. if a static member defines a
kernel we should import it.

The main benefit to this is that we can now link against the
`libomptarget.devicertl.a` library unconditionally. This removes the
requirement for users to specify LTO on the link command. This will also
allow us to stop using the `amdgcn` bitcode versions of the libraries.

  clang foo.c -fopenmp --offload-arch=gfx1030 -foffload-lto -c
  clang foo.o -fopenmp --offload-arch=gfx1030 -foffload-lto


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142484

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  openmp/libomptarget/test/offloading/static_linking.c

Index: openmp/libomptarget/test/offloading/static_linking.c
===================================================================
--- openmp/libomptarget/test/offloading/static_linking.c
+++ openmp/libomptarget/test/offloading/static_linking.c
@@ -1,6 +1,6 @@
 // RUN: %libomptarget-compile-generic -DLIBRARY -c -o %t.o
 // RUN: ar rcs %t.a %t.o
-// RUN: %libomptarget-compile-generic %t.a && %libomptarget-run-generic 2>&1 | %fcheck-generic
+// RUN: %libomptarget-compile-generic %t.a %t.a %t.a %t.a && %libomptarget-run-generic 2>&1 | %fcheck-generic
 
 #ifdef LIBRARY
 int x = 42;
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===================================================================
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -1136,9 +1136,108 @@
   return searchLibraryBaseName(Input, Root, SearchPaths);
 }
 
-/// Search the input files and libraries for embedded device offloading code and
-/// add it to the list of files to be linked. Files coming from static libraries
-/// are only added to the input if they are used by an existing input file.
+enum Symbol : uint32_t {
+  Sym_None = 0,
+  Sym_Undefined = 1U << 1,
+  Sym_Weak = 1U << 2,
+};
+
+Expected<bool> getSymbolsFromBitcode(MemoryBufferRef Buffer, StringSaver &Saver,
+                                     DenseMap<StringRef, Symbol> &Syms) {
+  Expected<IRSymtabFile> IRSymtabOrErr = readIRSymtab(Buffer);
+  if (!IRSymtabOrErr)
+    return IRSymtabOrErr.takeError();
+
+  bool Resolved = false;
+  for (unsigned I = 0; I != IRSymtabOrErr->Mods.size(); ++I) {
+    for (const auto &Sym : IRSymtabOrErr->TheReader.module_symbols(I)) {
+      if (Sym.isFormatSpecific() || !Sym.isGlobal())
+        continue;
+
+      bool NewSymbol = Syms.count(Sym.getName()) == 0;
+      auto &OldSym = Syms[Saver.save(Sym.getName())];
+
+      Resolved |= ((OldSym & Sym_Undefined && !(OldSym & Sym_Weak)) &&
+                   !Sym.isUndefined()) ||
+                  ((NewSymbol || (OldSym & Sym_Undefined)) &&
+                   !Sym.isUndefined() && !Sym.canBeOmittedFromSymbolTable() &&
+                   (Sym.getVisibility() != GlobalValue::HiddenVisibility));
+
+      if (OldSym & Sym_Undefined && !Sym.isUndefined())
+        OldSym = static_cast<Symbol>(OldSym & ~Sym_Undefined);
+      if (Sym.isUndefined() && NewSymbol)
+        OldSym = static_cast<Symbol>(OldSym | Sym_Undefined);
+      if (Sym.isWeak())
+        OldSym = static_cast<Symbol>(OldSym | Sym_Weak);
+    }
+  }
+
+  return Resolved;
+}
+
+Expected<bool> getSymbolsFromObject(const ObjectFile &Obj, StringSaver &Saver,
+                                    DenseMap<StringRef, Symbol> &Syms) {
+  bool Resolved = false;
+  for (SymbolRef Sym : Obj.symbols()) {
+    auto FlagsOrErr = Sym.getFlags();
+    if (!FlagsOrErr)
+      return FlagsOrErr.takeError();
+
+    if (!(*FlagsOrErr & SymbolRef::SF_Global) ||
+        (*FlagsOrErr & SymbolRef::SF_FormatSpecific))
+      continue;
+
+    auto NameOrErr = Sym.getName();
+    if (!NameOrErr)
+      return NameOrErr.takeError();
+
+    bool NewSymbol = Syms.count(*NameOrErr) == 0;
+    auto &OldSym = Syms[Saver.save(*NameOrErr)];
+
+    Resolved |= ((OldSym & Sym_Undefined && !(OldSym & Sym_Weak)) &&
+                 !(*FlagsOrErr & SymbolRef::SF_Undefined)) ||
+                ((NewSymbol || (OldSym & Sym_Undefined)) &&
+                 !(*FlagsOrErr & SymbolRef::SF_Undefined) &&
+                 !(*FlagsOrErr & SymbolRef::SF_Hidden));
+
+    if (OldSym & Sym_Undefined && !(*FlagsOrErr & SymbolRef::SF_Undefined))
+      OldSym = static_cast<Symbol>(OldSym & ~Sym_Undefined);
+    if (*FlagsOrErr & SymbolRef::SF_Undefined && NewSymbol)
+      OldSym = static_cast<Symbol>(OldSym | Sym_Undefined);
+    if (*FlagsOrErr & SymbolRef::SF_Weak)
+      OldSym = static_cast<Symbol>(OldSym | Sym_Weak);
+  }
+  return Resolved;
+}
+
+/// Attempt to 'resolve' symbols found in input files. We use this to
+/// determine if an archive member needs to be extracted. An archive member
+/// will be extracted if any of the following is true.
+///   1) It defines an undefined symbol in a regular object filie.
+///   2) It defines a global symbol without hidden visibility that has not
+///      yet been defined.
+Expected<bool> getSymbols(StringRef Image, StringSaver &Saver,
+                          DenseMap<StringRef, Symbol> &Syms) {
+  MemoryBufferRef Buffer = MemoryBufferRef(Image, "");
+  switch (identify_magic(Image)) {
+  case file_magic::bitcode:
+    return getSymbolsFromBitcode(Buffer, Saver, Syms);
+  case file_magic::elf_relocatable: {
+    Expected<std::unique_ptr<ObjectFile>> ObjFile =
+        ObjectFile::createObjectFile(Buffer);
+    if (!ObjFile)
+      return ObjFile.takeError();
+    return getSymbolsFromObject(**ObjFile, Saver, Syms);
+  }
+  default:
+    return false;
+  }
+}
+
+/// Search the input files and libraries for embedded device offloading code
+/// and add it to the list of files to be linked. Files coming from static
+/// libraries are only added to the input if they are used by an existing
+/// input file.
 Expected<SmallVector<OffloadFile>> getDeviceInput(const ArgList &Args) {
   llvm::TimeTraceScope TimeScope("ExtractDeviceCode");
 
@@ -1147,47 +1246,68 @@
   for (const opt::Arg *Arg : Args.filtered(OPT_library_path))
     LibraryPaths.push_back(Arg->getValue());
 
+  BumpPtrAllocator Alloc;
+  StringSaver Saver(Alloc);
+
   // Try to extract device code from the linker input files.
   SmallVector<OffloadFile> InputFiles;
-  SmallVector<OffloadFile> LazyInputFiles;
-  for (const opt::Arg *Arg : Args.filtered(OPT_INPUT)) {
-    StringRef Filename = Arg->getValue();
-    if (!sys::fs::exists(Filename) || sys::fs::is_directory(Filename))
+  DenseMap<OffloadFile::TargetID, DenseMap<StringRef, Symbol>> Syms;
+  for (const opt::Arg *Arg : Args.filtered(OPT_INPUT, OPT_library)) {
+    std::optional<std::string> Filename =
+        Arg->getOption().matches(OPT_library)
+            ? searchLibrary(Arg->getValue(), Root, LibraryPaths)
+            : std::string(Arg->getValue());
+
+    if (!Filename && Arg->getOption().matches(OPT_library))
+      reportError(createStringError(inconvertibleErrorCode(),
+                                    "unable to find library -l%s",
+                                    Arg->getValue()));
+
+    if (!Filename || !sys::fs::exists(*Filename) ||
+        sys::fs::is_directory(*Filename))
       continue;
 
     ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
-        MemoryBuffer::getFileOrSTDIN(Filename);
+        MemoryBuffer::getFileOrSTDIN(*Filename);
     if (std::error_code EC = BufferOrErr.getError())
-      return createFileError(Filename, EC);
+      return createFileError(*Filename, EC);
 
-    if (identify_magic((*BufferOrErr)->getBuffer()) ==
-        file_magic::elf_shared_object)
+    MemoryBufferRef Buffer = **BufferOrErr;
+    if (identify_magic(Buffer.getBuffer()) == file_magic::elf_shared_object)
       continue;
 
-    bool IsLazy =
-        identify_magic((*BufferOrErr)->getBuffer()) == file_magic::archive;
-    if (Error Err = extractOffloadBinaries(
-            **BufferOrErr, IsLazy ? LazyInputFiles : InputFiles))
+    SmallVector<OffloadFile> Binaries;
+    if (Error Err = extractOffloadBinaries(Buffer, Binaries))
       return std::move(Err);
-  }
 
-  // Try to extract input from input archive libraries.
-  for (const opt::Arg *Arg : Args.filtered(OPT_library)) {
-    if (auto Library = searchLibrary(Arg->getValue(), Root, LibraryPaths)) {
-      ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
-          MemoryBuffer::getFileOrSTDIN(*Library);
-      if (std::error_code EC = BufferOrErr.getError())
-        reportError(createFileError(*Library, EC));
-
-      if (identify_magic((*BufferOrErr)->getBuffer()) != file_magic::archive)
-        continue;
-
-      if (Error Err = extractOffloadBinaries(**BufferOrErr, LazyInputFiles))
-        return std::move(Err);
-    } else {
-      reportError(createStringError(inconvertibleErrorCode(),
-                                    "unable to find library -l%s",
-                                    Arg->getValue()));
+    // We only extract archive members that are needed.
+    bool IsArchive = identify_magic(Buffer.getBuffer()) == file_magic::archive;
+    bool Extracted = true;
+    while (Extracted) {
+      Extracted = false;
+      for (OffloadFile &Binary : Binaries) {
+        if (!Binary.getBinary())
+          continue;
+
+        // If we don't have an object file for this architecture do not
+        // extract.
+        if (IsArchive && !Syms.count(Binary))
+          continue;
+
+        Expected<bool> ResolvedOrErr =
+            getSymbols(Binary.getBinary()->getImage(), Saver, Syms[Binary]);
+        if (!ResolvedOrErr)
+          return ResolvedOrErr.takeError();
+
+        Extracted = IsArchive && *ResolvedOrErr;
+
+        if (!IsArchive || Extracted)
+          InputFiles.emplace_back(std::move(Binary));
+
+        // If we extracted any files we need to check all the symbols again.
+        if (Extracted)
+          break;
+      }
     }
   }
 
@@ -1198,16 +1318,6 @@
     InputFiles.push_back(std::move(*FileOrErr));
   }
 
-  DenseSet<OffloadFile::TargetID> IsTargetUsed;
-  for (const auto &File : InputFiles)
-    IsTargetUsed.insert(File);
-
-  // We should only include input files that are used.
-  // TODO: Only load a library if it defined undefined symbols in the input.
-  for (auto &LazyFile : LazyInputFiles)
-    if (IsTargetUsed.contains(LazyFile))
-      InputFiles.emplace_back(std::move(LazyFile));
-
   return std::move(InputFiles);
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to