[llvm] 673cfcd - Revert "[Linker] Propagate `nobuiltin` attributes when linking known libcalls (#89431)"

Joseph Huber via llvm-commits llvm-commits at lists.llvm.org
Thu May 9 07:24:34 PDT 2024


Author: Joseph Huber
Date: 2024-05-09T09:24:08-05:00
New Revision: 673cfcd03b7b938b422fee07d8ca4a127d480b1f

URL: https://github.com/llvm/llvm-project/commit/673cfcd03b7b938b422fee07d8ca4a127d480b1f
DIFF: https://github.com/llvm/llvm-project/commit/673cfcd03b7b938b422fee07d8ca4a127d480b1f.diff

LOG: Revert "[Linker] Propagate `nobuiltin` attributes when linking known libcalls (#89431)"

This apparently breaks AMDGPU offloading for unknown reasons. Reverting
for now.

This reverts commit aa16de6399a42421076ed642c3b4f7fb12c6d44b.

Added: 
    

Modified: 
    llvm/include/llvm/Linker/IRMover.h
    llvm/lib/Linker/CMakeLists.txt
    llvm/lib/Linker/IRMover.cpp

Removed: 
    llvm/test/Linker/Inputs/strlen.ll
    llvm/test/Linker/libcalls.ll


################################################################################
diff  --git a/llvm/include/llvm/Linker/IRMover.h b/llvm/include/llvm/Linker/IRMover.h
index 8e71c6080dffd..1e3c5394ffa2a 100644
--- a/llvm/include/llvm/Linker/IRMover.h
+++ b/llvm/include/llvm/Linker/IRMover.h
@@ -12,14 +12,11 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FunctionExtras.h"
-#include "llvm/ADT/StringSet.h"
-#include "llvm/IR/GlobalValue.h"
-#include "llvm/Support/StringSaver.h"
-#include "llvm/TargetParser/Triple.h"
 #include <functional>
 
 namespace llvm {
 class Error;
+class GlobalValue;
 class Metadata;
 class Module;
 class StructType;
@@ -63,33 +60,6 @@ class IRMover {
     bool hasType(StructType *Ty);
   };
 
-  /// Utility for handling linking of known libcall functions. If a merged
-  /// module contains a recognized library call we can no longer perform any
-  /// libcall related transformations.
-  class LibcallHandler {
-    bool HasLibcalls = false;
-
-    StringSet<> Libcalls;
-    StringSet<> Triples;
-
-    BumpPtrAllocator Alloc;
-    StringSaver Saver;
-
-  public:
-    LibcallHandler() : Saver(Alloc) {}
-
-    void updateLibcalls(const Triple &TheTriple);
-
-    bool checkLibcalls(GlobalValue &GV) {
-      if (HasLibcalls)
-        return false;
-      return HasLibcalls = isa<Function>(&GV) && !GV.isDeclaration() &&
-                           Libcalls.count(GV.getName());
-    }
-
-    bool hasLibcalls() const { return HasLibcalls; }
-  };
-
   IRMover(Module &M);
 
   typedef std::function<void(GlobalValue &)> ValueAdder;
@@ -114,7 +84,6 @@ class IRMover {
   Module &Composite;
   IdentifiedStructTypeSet IdentifiedStructTypes;
   MDMapT SharedMDs; ///< A Metadata map to use for all calls to \a move().
-  LibcallHandler Libcalls;
 };
 
 } // End llvm namespace

diff  --git a/llvm/lib/Linker/CMakeLists.txt b/llvm/lib/Linker/CMakeLists.txt
index 25001c09a62d2..5afb40f8b5884 100644
--- a/llvm/lib/Linker/CMakeLists.txt
+++ b/llvm/lib/Linker/CMakeLists.txt
@@ -9,7 +9,6 @@ add_llvm_component_library(LLVMLinker
   intrinsics_gen
 
   LINK_COMPONENTS
-  Analysis
   Core
   Object
   Support

diff  --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index fe2b531835895..7a5aa0c804782 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -12,7 +12,6 @@
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfoMetadata.h"
@@ -400,9 +399,6 @@ class IRLinker {
   /// A metadata map that's shared between IRLinker instances.
   MDMapT &SharedMDs;
 
-  /// A list of libcalls that the current target may call.
-  IRMover::LibcallHandler &Libcalls;
-
   /// Mapping of values from what they used to be in Src, to what they are now
   /// in DstM.  ValueToValueMapTy is a ValueMap, which involves some overhead
   /// due to the use of Value handles which the Linker doesn't actually need,
@@ -544,12 +540,10 @@ class IRLinker {
   IRLinker(Module &DstM, MDMapT &SharedMDs,
            IRMover::IdentifiedStructTypeSet &Set, std::unique_ptr<Module> SrcM,
            ArrayRef<GlobalValue *> ValuesToLink,
-           IRMover::LibcallHandler &Libcalls, IRMover::LazyCallback AddLazyFor,
-           bool IsPerformingImport)
+           IRMover::LazyCallback AddLazyFor, bool IsPerformingImport)
       : DstM(DstM), SrcM(std::move(SrcM)), AddLazyFor(std::move(AddLazyFor)),
         TypeMap(Set), GValMaterializer(*this), LValMaterializer(*this),
-        SharedMDs(SharedMDs), Libcalls(Libcalls),
-        IsPerformingImport(IsPerformingImport),
+        SharedMDs(SharedMDs), IsPerformingImport(IsPerformingImport),
         Mapper(ValueMap, RF_ReuseAndMutateDistinctMDs | RF_IgnoreMissingLocals,
                &TypeMap, &GValMaterializer),
         IndirectSymbolMCID(Mapper.registerAlternateMappingContext(
@@ -567,13 +561,6 @@ class IRLinker {
 };
 }
 
-static void addNoBuiltinAttributes(Function &F) {
-  F.setAttributes(
-      F.getAttributes().addFnAttribute(F.getContext(), "no-builtins"));
-  F.setAttributes(
-      F.getAttributes().addFnAttribute(F.getContext(), Attribute::NoBuiltin));
-}
-
 /// The LLVM SymbolTable class autorenames globals that conflict in the symbol
 /// table. This is good for all clients except for us. Go through the trouble
 /// to force this back.
@@ -1618,26 +1605,14 @@ Error IRLinker::run() {
 
   DstM.setTargetTriple(SrcTriple.merge(DstTriple));
 
-  // Update the target triple's libcall information if it was changed.
-  Libcalls.updateLibcalls(Triple(DstM.getTargetTriple()));
-
   // Loop over all of the linked values to compute type mappings.
   computeTypeMapping();
 
-  bool AddsLibcalls = false;
   std::reverse(Worklist.begin(), Worklist.end());
   while (!Worklist.empty()) {
     GlobalValue *GV = Worklist.back();
     Worklist.pop_back();
 
-    // If the module already contains libcall functions we need every function
-    // linked in to have `nobuiltin` attributes. Otherwise check if this is a
-    // libcall definition.
-    if (Function *F = dyn_cast<Function>(GV); F && Libcalls.hasLibcalls())
-      addNoBuiltinAttributes(*F);
-    else
-      AddsLibcalls = Libcalls.checkLibcalls(*GV);
-
     // Already mapped.
     if (ValueMap.find(GV) != ValueMap.end() ||
         IndirectSymbolValueMap.find(GV) != IndirectSymbolValueMap.end())
@@ -1700,13 +1675,6 @@ Error IRLinker::run() {
     }
   }
 
-  // If we have imported a recognized libcall function we can no longer make any
-  // reasonable optimizations based off of its semantics. Add the 'nobuiltin'
-  // attribute to every function to suppress libcall detection.
-  if (AddsLibcalls)
-    for (Function &F : DstM.functions())
-      addNoBuiltinAttributes(F);
-
   // Merge the module flags into the DstM module.
   return linkModuleFlagsMetadata();
 }
@@ -1789,22 +1757,6 @@ bool IRMover::IdentifiedStructTypeSet::hasType(StructType *Ty) {
   return I == NonOpaqueStructTypes.end() ? false : *I == Ty;
 }
 
-void IRMover::LibcallHandler::updateLibcalls(const Triple &TheTriple) {
-  if (Triples.count(TheTriple.getTriple()))
-    return;
-  Triples.insert(Saver.save(TheTriple.getTriple()));
-
-  // Collect the names of runtime functions that the target may want to call.
-  TargetLibraryInfoImpl TLII(TheTriple);
-  TargetLibraryInfo TLI(TLII);
-  for (unsigned I = 0, E = static_cast<unsigned>(LibFunc::NumLibFuncs); I != E;
-       ++I) {
-    LibFunc F = static_cast<LibFunc>(I);
-    if (TLI.has(F))
-      Libcalls.insert(TLI.getName(F));
-  }
-}
-
 IRMover::IRMover(Module &M) : Composite(M) {
   TypeFinder StructTypes;
   StructTypes.run(M, /* OnlyNamed */ false);
@@ -1820,25 +1772,14 @@ IRMover::IRMover(Module &M) : Composite(M) {
   for (const auto *MD : StructTypes.getVisitedMetadata()) {
     SharedMDs[MD].reset(const_cast<MDNode *>(MD));
   }
-
-  // Check the composite module for any already present libcalls. If we define
-  // these then it is important to mark any imported functions as 'nobuiltin'.
-  Libcalls.updateLibcalls(Triple(Composite.getTargetTriple()));
-  for (Function &F : Composite.functions())
-    if (Libcalls.checkLibcalls(F))
-      break;
-
-  if (Libcalls.hasLibcalls())
-    for (Function &F : Composite.functions())
-      addNoBuiltinAttributes(F);
 }
 
 Error IRMover::move(std::unique_ptr<Module> Src,
                     ArrayRef<GlobalValue *> ValuesToLink,
                     LazyCallback AddLazyFor, bool IsPerformingImport) {
   IRLinker TheIRLinker(Composite, SharedMDs, IdentifiedStructTypes,
-                       std::move(Src), ValuesToLink, Libcalls,
-                       std::move(AddLazyFor), IsPerformingImport);
+                       std::move(Src), ValuesToLink, std::move(AddLazyFor),
+                       IsPerformingImport);
   Error E = TheIRLinker.run();
   Composite.dropTriviallyDeadConstantArrays();
   return E;

diff  --git a/llvm/test/Linker/Inputs/strlen.ll b/llvm/test/Linker/Inputs/strlen.ll
deleted file mode 100644
index bc54aaf41e0ce..0000000000000
--- a/llvm/test/Linker/Inputs/strlen.ll
+++ /dev/null
@@ -1,21 +0,0 @@
-target triple = "x86_64-unknown-linux-gnu"
-
-define i64 @strlen(ptr %s) #0 {
-entry:
-  br label %for.cond
-
-for.cond:
-  %s.addr.0 = phi ptr [ %s, %entry ], [ %incdec.ptr, %for.cond ]
-  %0 = load i8, ptr %s.addr.0, align 1
-  %tobool.not = icmp eq i8 %0, 0
-  %incdec.ptr = getelementptr inbounds i8, ptr %s.addr.0, i64 1
-  br i1 %tobool.not, label %for.end, label %for.cond
-
-for.end:
-  %sub.ptr.lhs.cast = ptrtoint ptr %s.addr.0 to i64
-  %sub.ptr.rhs.cast = ptrtoint ptr %s to i64
-  %sub.ptr.sub = sub i64 %sub.ptr.lhs.cast, %sub.ptr.rhs.cast
-  ret i64 %sub.ptr.sub
-}
-
-attributes #0 = { noinline }

diff  --git a/llvm/test/Linker/libcalls.ll b/llvm/test/Linker/libcalls.ll
deleted file mode 100644
index ddc0d35e91d9e..0000000000000
--- a/llvm/test/Linker/libcalls.ll
+++ /dev/null
@@ -1,39 +0,0 @@
-; RUN: llvm-link %s %S/Inputs/strlen.ll -S -o - 2>%t.a.err | FileCheck %s --check-prefix=CHECK1
-; RUN: llvm-link %S/Inputs/strlen.ll %s -S -o - 2>%t.a.err | FileCheck %s --check-prefix=CHECK2
-
-target triple = "x86_64-unknown-linux-gnu"
-
- at .str = private unnamed_addr constant [7 x i8] c"string\00", align 1
- at str = dso_local global ptr @.str, align 8
-
-define void @foo() #0 {
-  ret void
-}
-
-declare i64 @strlen(ptr)
-
-define void @bar() #0 {
-  ret void
-}
-
-define i64 @baz() #0 {
-entry:
-  %0 = load ptr, ptr @str, align 8
-  %call = call i64 @strlen(ptr noundef %0)
-  ret i64 %call
-}
-
-attributes #0 = { noinline }
-
-; CHECK1: define void @foo() #[[ATTR0:[0-9]+]]
-; CHECK1: define void @bar() #[[ATTR0:[0-9]+]]
-; CHECK1: define i64 @baz() #[[ATTR0:[0-9]+]]
-; CHECK1: define i64 @strlen(ptr [[S:%.*]]) #[[ATTR0]]
-
-; CHECK2: define i64 @strlen(ptr [[S:%.*]]) #[[ATTR0:[0-9]+]]
-; CHECK2: define void @foo() #[[ATTR0:[0-9]+]]
-; CHECK2: define void @bar() #[[ATTR0:[0-9]+]]
-; CHECK2: define i64 @baz() #[[ATTR0]]
-
-; CHECK1: attributes #[[ATTR0]] = { nobuiltin noinline "no-builtins" }
-; CHECK2: attributes #[[ATTR0]] = { nobuiltin noinline "no-builtins" }


        


More information about the llvm-commits mailing list