Skip to content

Conversation

Arghnews
Copy link
Contributor

@Arghnews Arghnews commented Sep 9, 2025

Also fixes (#131488)

Unreachable case is triggering Callees.empty() assert. Since this was originally a continue anyway, have applied that as a fix and added a test case. Please let me know if there's a better way.

Not sure who/how to get folks to review, tagging a few people (apologies if you're not the right person/this is the wrong way to do it, please let me know what to do in future if so)

@labrinea @dtcxzyw @nikic @fhahn

@llvmbot
Copy link
Member

llvmbot commented Sep 9, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Justin Riddell (Arghnews)

Changes

Also fixes (#131488)

Unreachable case is triggering Callees.empty() assert. Since this was originally a continue anyway, have applied that as a fix and added a test case


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/GlobalOpt.cpp (+2-1)
  • (added) llvm/test/Transforms/GlobalOpt/resolve-indirect-ifunc.ll (+11)
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index d7edd1288309b..f88d51f443bcf 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -2551,7 +2551,8 @@ static bool OptimizeNonTrivialIFuncs(
         }))
       continue;
 
-    assert(!Callees.empty() && "Expecting successful collection of versions");
+    if (Callees.empty())
+      continue;
 
     LLVM_DEBUG(dbgs() << "Statically resolving calls to function "
                       << Resolver->getName() << "\n");
diff --git a/llvm/test/Transforms/GlobalOpt/resolve-indirect-ifunc.ll b/llvm/test/Transforms/GlobalOpt/resolve-indirect-ifunc.ll
new file mode 100644
index 0000000000000..1f944a7bde19b
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/resolve-indirect-ifunc.ll
@@ -0,0 +1,11 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt --passes=globalopt -o - -S < %s | FileCheck %s
+
+define ptr @f1() partition "part1" {
+; CHECK-LABEL: define ptr @f1() partition "part1" {
+; CHECK-NEXT:    unreachable
+;
+  unreachable
+}
+
+@i1 = ifunc void(), ptr @f1, partition "part2"

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@Arghnews Arghnews force-pushed the opt_empty_callees_crash branch from 1f439e4 to 05ea023 Compare September 9, 2025 23:01
@Arghnews
Copy link
Contributor Author

Arghnews commented Sep 9, 2025

Fyi @nikic I don't have write access, will need someone to commit for me, thanks

@hstk30-hw hstk30-hw merged commit cc05266 into llvm:main Sep 10, 2025
9 checks passed
@nikic
Copy link
Contributor

nikic commented Sep 10, 2025

@hstk30-hw Please ensure that you drop any mentions from the commit description when merging changes. We'll be getting these notifications forever now.

@hstk30-hw
Copy link
Contributor

Okay, thanks for your reminder. I'll be more mindful next time

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

Successfully merging this pull request may close these issues.

5 participants