Skip to content

[BOLT][NFC] Add HasInternalCalls BinaryFunction property #90804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aaupov
Copy link
Contributor

@aaupov aaupov commented May 2, 2024

Use HasInternalCalls in ValidateInternalCalls to avoid re-checking all
functions.

Test Plan: NFC

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented May 2, 2024

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

This is a prerequisite for checking the reason why the function is
non-simple. Also use HasInternalCalls in ValidateInternalCalls to avoid
re-checking all functions.

Test Plan: NFC


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

3 Files Affected:

  • (modified) bolt/include/bolt/Core/BinaryFunction.h (+6)
  • (modified) bolt/lib/Core/BinaryFunction.cpp (+1)
  • (modified) bolt/lib/Passes/ValidateInternalCalls.cpp (+4-9)
diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index 26d2d01f862671..540a9767760ad9 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -361,6 +361,9 @@ class BinaryFunction {
   /// True if another function body was merged into this one.
   bool HasFunctionsFoldedInto{false};
 
+  /// True if the function has internal calls.
+  bool HasInternalCalls{false};
+
   /// Name for the section this function code should reside in.
   std::string CodeSectionName;
 
@@ -1334,6 +1337,9 @@ class BinaryFunction {
   /// Return true if other functions were folded into this one.
   bool hasFunctionsFoldedInto() const { return HasFunctionsFoldedInto; }
 
+  /// Return true if the function has internal calls.
+  bool hasInternalCalls() const { return HasInternalCalls; }
+
   /// If this function was folded, return the function it was folded into.
   BinaryFunction *getFoldedIntoFunction() const { return FoldedIntoFunction; }
 
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 1fa96dfaabde81..fff53dd25004a4 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1281,6 +1281,7 @@ Error BinaryFunction::disassemble() {
             // Recursive call.
             TargetSymbol = getSymbol();
           } else {
+            HasInternalCalls = true;
             if (BC.isX86()) {
               // Dangerous old-style x86 PIC code. We may need to freeze this
               // function, so preserve the function as is for now.
diff --git a/bolt/lib/Passes/ValidateInternalCalls.cpp b/bolt/lib/Passes/ValidateInternalCalls.cpp
index 88df2e5b59f389..24f9bfde401ae9 100644
--- a/bolt/lib/Passes/ValidateInternalCalls.cpp
+++ b/bolt/lib/Passes/ValidateInternalCalls.cpp
@@ -309,15 +309,10 @@ Error ValidateInternalCalls::runOnFunctions(BinaryContext &BC) {
   std::set<BinaryFunction *> NeedsValidation;
   for (auto &BFI : BC.getBinaryFunctions()) {
     BinaryFunction &Function = BFI.second;
-    for (BinaryBasicBlock &BB : Function) {
-      for (MCInst &Inst : BB) {
-        if (getInternalCallTarget(Function, Inst)) {
-          NeedsValidation.insert(&Function);
-          Function.setSimple(false);
-          break;
-        }
-      }
-    }
+    if (!Function.hasInternalCalls())
+      continue;
+    NeedsValidation.insert(&Function);
+    Function.setSimple(false);
   }
 
   // Skip validation for non-relocation mode

@aaupov aaupov marked this pull request as draft May 5, 2024 20:42
@aaupov aaupov marked this pull request as ready for review May 8, 2024 20:49
Copy link
Contributor

@maksfb maksfb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants