[llvm] PlaceSafepoints: Fix using default constructed TargetLibraryInfo (PR #92411)

[Matt Arsenault via llvm-commits]

https://github.com/arsenm created https://github.com/llvm/llvm-project/pull/92411

PlaceSafepoints has an awful hack where it's creating a legacy
PassManager inside it's runImpl, which was not propagating the incoming
TLI. This means there's an implicit bug fix, where PlaceSafepoints would have
 been treating too many calls as builtins.

Split from #92400 

>From 57a58e057d3af1b4208a74c89af26176c959fac8 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Thu, 16 May 2024 17:04:24 +0200
Subject: [PATCH 1/2] PlaceSafepoints: Add baseline test that shows
 TargetLibraryInfo is wrong

---
 llvm/test/Transforms/PlaceSafepoints/libcall.ll | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/llvm/test/Transforms/PlaceSafepoints/libcall.ll b/llvm/test/Transforms/PlaceSafepoints/libcall.ll
index 89090ba1b243c..fa1ddde62121e 100644
--- a/llvm/test/Transforms/PlaceSafepoints/libcall.ll
+++ b/llvm/test/Transforms/PlaceSafepoints/libcall.ll
@@ -1,5 +1,8 @@
 ; RUN: opt -S -passes=place-safepoints < %s | FileCheck %s
 
+; FIXME: This should fail without the ldexp builtin
+; RUN: opt -S -passes=place-safepoints -disable-builtin=ldexp < %s | FileCheck %s
+
 ; Libcalls will not contain a safepoint poll, so check that we insert
 ; a safepoint in a loop containing a libcall.
 declare double @ldexp(double %x, i32 %n) nounwind readnone

>From 1e2ec61e82cb0946a953edf8bed40f4b552852b2 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Thu, 16 May 2024 17:05:15 +0200
Subject: [PATCH 2/2] PlaceSafepoints: Fix using default constructed
 TargetLibraryInfo

PlaceSafepoints has an awful hack where it's creating a legacy
PassManager inside it's runImpl, which was not propagating the incoming
TLI. This means there's an implicit bug fix, where PlaceSafepoints would have
been treating too many calls as builtins.
---
 llvm/include/llvm/Analysis/TargetLibraryInfo.h  | 4 ++++
 llvm/lib/Analysis/TargetLibraryInfo.cpp         | 4 ++++
 llvm/lib/Transforms/Scalar/PlaceSafepoints.cpp  | 2 ++
 llvm/test/Transforms/PlaceSafepoints/libcall.ll | 8 ++++----
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.h b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
index 46f31f918e7b6..f5da222d11f55 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.h
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
@@ -633,6 +633,10 @@ class TargetLibraryInfoWrapperPass : public ImmutablePass {
   explicit TargetLibraryInfoWrapperPass(const Triple &T);
   explicit TargetLibraryInfoWrapperPass(const TargetLibraryInfoImpl &TLI);
 
+  // FIXME: This should be removed when PlaceSafepoints is fixed to not create a
+  // PassManager inside a pass.
+  explicit TargetLibraryInfoWrapperPass(const TargetLibraryInfo &TLI);
+
   TargetLibraryInfo &getTLI(const Function &F) {
     FunctionAnalysisManager DummyFAM;
     TLI = TLA.run(F, DummyFAM);
diff --git a/llvm/lib/Analysis/TargetLibraryInfo.cpp b/llvm/lib/Analysis/TargetLibraryInfo.cpp
index c62d9daa13ef0..684b0759f3157 100644
--- a/llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ b/llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -1383,6 +1383,10 @@ TargetLibraryInfoWrapperPass::TargetLibraryInfoWrapperPass(
   initializeTargetLibraryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
 }
 
+TargetLibraryInfoWrapperPass::TargetLibraryInfoWrapperPass(
+    const TargetLibraryInfo &TLIOther)
+    : TargetLibraryInfoWrapperPass(*TLIOther.Impl) {}
+
 AnalysisKey TargetLibraryAnalysis::Key;
 
 // Register the basic pass.
diff --git a/llvm/lib/Transforms/Scalar/PlaceSafepoints.cpp b/llvm/lib/Transforms/Scalar/PlaceSafepoints.cpp
index 77d155d7e78e3..dcdea6e7b62ae 100644
--- a/llvm/lib/Transforms/Scalar/PlaceSafepoints.cpp
+++ b/llvm/lib/Transforms/Scalar/PlaceSafepoints.cpp
@@ -288,6 +288,8 @@ bool PlaceSafepointsPass::runImpl(Function &F, const TargetLibraryInfo &TLI) {
     // with for the moment.
     legacy::FunctionPassManager FPM(F.getParent());
     bool CanAssumeCallSafepoints = enableCallSafepoints(F);
+
+    FPM.add(new TargetLibraryInfoWrapperPass(TLI));
     auto *PBS = new PlaceBackedgeSafepointsLegacyPass(CanAssumeCallSafepoints);
     FPM.add(PBS);
     FPM.run(F);
diff --git a/llvm/test/Transforms/PlaceSafepoints/libcall.ll b/llvm/test/Transforms/PlaceSafepoints/libcall.ll
index fa1ddde62121e..6e26f924a5ba3 100644
--- a/llvm/test/Transforms/PlaceSafepoints/libcall.ll
+++ b/llvm/test/Transforms/PlaceSafepoints/libcall.ll
@@ -1,8 +1,7 @@
-; RUN: opt -S -passes=place-safepoints < %s | FileCheck %s
-
-; FIXME: This should fail without the ldexp builtin
+; RUN: opt -S -passes=place-safepoints < %s | FileCheck -check-prefixes=CHECK,WITHLDEXPF %s
 ; RUN: opt -S -passes=place-safepoints -disable-builtin=ldexp < %s | FileCheck %s
 
+
 ; Libcalls will not contain a safepoint poll, so check that we insert
 ; a safepoint in a loop containing a libcall.
 declare double @ldexp(double %x, i32 %n) nounwind readnone
@@ -20,7 +19,8 @@ loop:
 ; CHECK-NEXT: %x_loop = phi double [ %x, %entry ], [ %x_exp, %loop ]
 ; CHECK-NEXT: %x_exp = call double @ldexp(double %x_loop, i32 5)
 ; CHECK-NEXT: %done = fcmp ogt double %x_exp, 1.5
-; CHECK-NEXT: call void @do_safepoint
+; WITHLDEXPF-NEXT: call void @do_safepoint
+; CHECK-NEXT: br
   %x_loop = phi double [ %x, %entry ], [ %x_exp, %loop ]
   %x_exp = call double @ldexp(double %x_loop, i32 5) nounwind readnone
   %done = fcmp ogt double %x_exp, 1.5