Add new helper Module::getRequiredFunction.

[tsymalla]

Sometimes the existance of a function on a module is strictly required, thus the caller of Module::getFunction needs to handle the possible bailing out.

Add a new helper, Module::getRequiredFunction, that adds an additional check.

In the RewriteDescriptor, the presence of only a single argument to getFunction is required, thus adding a new helper function.

Since we cannot overload functions on the basis of a default value for an argument, I just added a new helper.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-ir

Author: Thomas Symalla (tsymalla)

Changes

Sometimes the existance of a function on a module is strictly required, thus the caller of Module::getFunction needs to handle the possible bailing out.

Add a new helper, Module::getRequiredFunction, that adds an additional check.

In the RewriteDescriptor, the presence of only a single argument to getFunction is required, thus adding a new helper function.

Since we cannot overload functions on the basis of a default value for an argument, I just added a new helper.

---

Full diff: https://github.com/llvm/llvm-project/pull/82761.diff

4 Files Affected:

• (modified) llvm/include/llvm/IR/Module.h (+4)
• (modified) llvm/lib/IR/Module.cpp (+12)
• (modified) llvm/unittests/Analysis/CFGTest.cpp (+1-3)
• (modified) llvm/unittests/Analysis/VectorUtilsTest.cpp (+1-3)

diff --git a/llvm/include/llvm/IR/Module.h b/llvm/include/llvm/IR/Module.h
index 68a89dc45c2834..db4df92cd135d5 100644
--- a/llvm/include/llvm/IR/Module.h
+++ b/llvm/include/llvm/IR/Module.h
@@ -424,6 +424,10 @@ class LLVM_EXTERNAL_VISIBILITY Module {
   /// exist, return null.
   Function *getFunction(StringRef Name) const;
 
+  /// Look up the specified function in the module symbol table.
+  /// If the function does not exist, then bail out.
+  Function *getRequiredFunction(StringRef Name) const;
+
 /// @}
 /// @name Global Variable Accessors
 /// @{
diff --git a/llvm/lib/IR/Module.cpp b/llvm/lib/IR/Module.cpp
index 1946db2ee0be7e..1d40ba083c789f 100644
--- a/llvm/lib/IR/Module.cpp
+++ b/llvm/lib/IR/Module.cpp
@@ -170,6 +170,18 @@ Function *Module::getFunction(StringRef Name) const {
   return dyn_cast_or_null<Function>(getNamedValue(Name));
 }
 
+// getRequiredFunction - Look up the specified function in the module symbol
+// table. If the function does not exist, bail out.
+//
+Function *Module::getRequiredFunction(StringRef Name) const {
+  Function *F = getFunction(Name);
+  if (!F)
+    report_fatal_error(Twine("Required function '@") + Name +
+                       "' not found on module '" + getName() + "'!");
+
+  return F;
+}
+
 //===----------------------------------------------------------------------===//
 // Methods for easy access to the global variables in the module.
 //
diff --git a/llvm/unittests/Analysis/CFGTest.cpp b/llvm/unittests/Analysis/CFGTest.cpp
index 46164268468628..c822ff4b52f7fd 100644
--- a/llvm/unittests/Analysis/CFGTest.cpp
+++ b/llvm/unittests/Analysis/CFGTest.cpp
@@ -41,9 +41,7 @@ class IsPotentiallyReachableTest : public testing::Test {
     if (!M)
       report_fatal_error(os.str().c_str());
 
-    Function *F = M->getFunction("test");
-    if (F == nullptr)
-      report_fatal_error("Test must have a function named @test");
+    Function *F = M->getRequiredFunction("test");
 
     A = B = nullptr;
     for (inst_iterator I = inst_begin(F), E = inst_end(F); I != E; ++I) {
diff --git a/llvm/unittests/Analysis/VectorUtilsTest.cpp b/llvm/unittests/Analysis/VectorUtilsTest.cpp
index 14958aa646a04d..08d7a85ef4fcb0 100644
--- a/llvm/unittests/Analysis/VectorUtilsTest.cpp
+++ b/llvm/unittests/Analysis/VectorUtilsTest.cpp
@@ -37,9 +37,7 @@ class VectorUtilsTest : public testing::Test {
     if (!M)
       report_fatal_error(Twine(os.str()));
 
-    Function *F = M->getFunction("test");
-    if (F == nullptr)
-      report_fatal_error("Test must have a function named @test");
+    Function *F = M->getRequiredFunction("test");
 
     A = nullptr;
     for (inst_iterator I = inst_begin(F), E = inst_end(F); I != E; ++I) {

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff f1e0392b822e06f39c49df3ba594f4c98f608ba0 fb9a75f9b1a8a56b85455e32dda6cdf2d01d3255 -- llvm/include/llvm/IR/Module.h llvm/lib/IR/Module.cpp llvm/unittests/Analysis/CFGTest.cpp llvm/unittests/Analysis/VectorUtilsTest.cpp

View the diff from clang-format here.

diff --git a/llvm/include/llvm/IR/Module.h b/llvm/include/llvm/IR/Module.h
index db4df92cd1..ec0ef67147 100644
--- a/llvm/include/llvm/IR/Module.h
+++ b/llvm/include/llvm/IR/Module.h
@@ -428,9 +428,9 @@ public:
   /// If the function does not exist, then bail out.
   Function *getRequiredFunction(StringRef Name) const;
 
-/// @}
-/// @name Global Variable Accessors
-/// @{
+  /// @}
+  /// @name Global Variable Accessors
+  /// @{
 
   /// Look up the specified global variable in the module symbol table. If it
   /// does not exist, return null. If AllowInternal is set to true, this

[phyBrackets]

Looks good to me, I think the PR test failure is unrelated? And there are some format issues that needs to get resolved. Thank You.

[aeubanks]

I'm not sure this is worth adding to Module as opposed to just handling a null return from getFunction() at the callsite.

In the RewriteDescriptor, the presence of only a single argument to getFunction is required, thus adding a new helper function.

Why is this?