Skip to content

[GlobalOpt] Don't resolve aliased ifuncs with undefined resolvees. #96220

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

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

labrinea
Copy link
Collaborator

Fixes #96197.

A global alias should always point to a definition. Ifuncs are definitions, so far so good. However an ifunc may be statically resolved to a function that is declared but not defined in the translation unit.

With this patch we perform static resolution if:

  • the resolvee is defined
  • otherwise none of the ifunc users is a global alias

Fixes llvm#96197.

A global alias should always point to a definition. Ifuncs are
definitions, so far so good. However an ifunc may be statically
resolved to a function that is declared but not defined in the
translation unit.

With this patch we perform static resolution if:
 * the resolvee is defined
 * otherwise none of the ifunc users is a global alias
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Alexandros Lamprineas (labrinea)

Changes

Fixes #96197.

A global alias should always point to a definition. Ifuncs are definitions, so far so good. However an ifunc may be statically resolved to a function that is declared but not defined in the translation unit.

With this patch we perform static resolution if:

  • the resolvee is defined
  • otherwise none of the ifunc users is a global alias

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

2 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/GlobalOpt.cpp (+3-1)
  • (modified) llvm/test/Transforms/GlobalOpt/resolve-static-ifunc.ll (+22)
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index b7c6d25657bc0..411a8454bf18b 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -2458,7 +2458,9 @@ static bool OptimizeStaticIFuncs(Module &M) {
   bool Changed = false;
   for (GlobalIFunc &IF : M.ifuncs())
     if (Function *Callee = hasSideeffectFreeStaticResolution(IF))
-      if (!IF.use_empty()) {
+      if (!IF.use_empty() &&
+          (!Callee->isDeclaration() ||
+           none_of(IF.users(), [](User *U) { return isa<GlobalAlias>(U); }))) {
         IF.replaceAllUsesWith(Callee);
         NumIFuncsResolved++;
         Changed = true;
diff --git a/llvm/test/Transforms/GlobalOpt/resolve-static-ifunc.ll b/llvm/test/Transforms/GlobalOpt/resolve-static-ifunc.ll
index 2a1717304fb4c..de087343208c7 100644
--- a/llvm/test/Transforms/GlobalOpt/resolve-static-ifunc.ll
+++ b/llvm/test/Transforms/GlobalOpt/resolve-static-ifunc.ll
@@ -7,11 +7,15 @@ target triple = "aarch64-unknown-linux-gnu"
 @trivial.ifunc = internal ifunc void (), ptr @trivial.resolver
 ;.
 ; CHECK: @unknown_condition = external local_unnamed_addr global i1
+; CHECK: @alias_decl = weak_odr alias void (), ptr @aliased_decl.ifunc
+; CHECK: @alias_def = weak_odr alias void (), ptr @aliased_def._Msimd
 ; CHECK: @external_ifunc.ifunc = dso_local ifunc void (), ptr @external_ifunc.resolver
 ; CHECK: @complex.ifunc = internal ifunc void (), ptr @complex.resolver
 ; CHECK: @sideeffects.ifunc = internal ifunc void (), ptr @sideeffects.resolver
 ; CHECK: @interposable_ifunc.ifunc = internal ifunc void (), ptr @interposable_ifunc.resolver
 ; CHECK: @interposable_resolver.ifunc = weak ifunc void (), ptr @interposable_resolver.resolver
+; CHECK: @aliased_decl.ifunc = weak_odr ifunc void (), ptr @aliased_decl.resolver
+; CHECK: @aliased_def.ifunc = weak_odr ifunc void (), ptr @aliased_def.resolver
 ;.
 define ptr @trivial.resolver() {
   ret ptr @trivial._Msimd
@@ -89,6 +93,20 @@ define void @interposable_resolver.default() {
   ret void
 }
 
+@alias_decl = weak_odr alias void (), ptr @aliased_decl.ifunc
+@aliased_decl.ifunc = weak_odr ifunc void (), ptr @aliased_decl.resolver
+declare void @aliased_decl._Msimd()
+define ptr @aliased_decl.resolver() {
+  ret ptr @aliased_decl._Msimd
+}
+
+@alias_def = weak_odr alias void (), ptr @aliased_def.ifunc
+@aliased_def.ifunc = weak_odr ifunc void (), ptr @aliased_def.resolver
+define void @aliased_def._Msimd() { ret void }
+define ptr @aliased_def.resolver() {
+  ret ptr @aliased_def._Msimd
+}
+
 define void @caller() {
 ; CHECK-LABEL: define void @caller() local_unnamed_addr {
 ; CHECK-NEXT:    call void @trivial._Msimd()
@@ -97,6 +115,8 @@ define void @caller() {
 ; CHECK-NEXT:    call void @sideeffects.ifunc()
 ; CHECK-NEXT:    call void @interposable_ifunc.ifunc()
 ; CHECK-NEXT:    call void @interposable_resolver.ifunc()
+; CHECK-NEXT:    call void @aliased_decl.ifunc()
+; CHECK-NEXT:    call void @aliased_def._Msimd()
 ; CHECK-NEXT:    ret void
 ;
   call void @trivial.ifunc()
@@ -105,5 +125,7 @@ define void @caller() {
   call void @sideeffects.ifunc()
   call void @interposable_ifunc.ifunc()
   call void @interposable_resolver.ifunc()
+  call void @aliased_decl.ifunc()
+  call void @aliased_def.ifunc()
   ret void
 }

@labrinea labrinea changed the title [GlobalOpt] Don't resolve aliased ifuncs with declared resolvees. [GlobalOpt] Don't resolve aliased ifuncs with undefined resolvees. Jun 20, 2024
Copy link
Contributor

@jroelofs jroelofs left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@labrinea labrinea merged commit 7c946f0 into llvm:main Jun 21, 2024
9 checks passed
@labrinea labrinea deleted the globalopt branch June 21, 2024 08:33
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jun 21, 2024

LLVM Buildbot has detected a new failure on builder clang-x86_64-debian-fast running on gribozavr4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/582

Here is the relevant piece of the build log for the reference:

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'Clang :: CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 3: /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -cc1 -internal-isystem /b/1/clang-x86_64-debian-fast/llvm.obj/lib/clang/19/include -nostdsysteminc -fclang-abi-compat=latest -triple aarch64 -target-feature +sve -target-feature +bf16 -disable-O0-optnone -Werror -Wall -emit-llvm -o - /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/opt -S -passes=mem2reg,tailcallelim | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -cc1 -internal-isystem /b/1/clang-x86_64-debian-fast/llvm.obj/lib/clang/19/include -nostdsysteminc -fclang-abi-compat=latest -triple aarch64 -target-feature +sve -target-feature +bf16 -disable-O0-optnone -Werror -Wall -emit-llvm -o - /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/opt -S -passes=mem2reg,tailcallelim
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
RUN: at line 4: /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -cc1 -internal-isystem /b/1/clang-x86_64-debian-fast/llvm.obj/lib/clang/19/include -nostdsysteminc -fclang-abi-compat=latest -triple aarch64 -target-feature +sve -target-feature +bf16 -disable-O0-optnone -Werror -Wall -emit-llvm -o - -x c++ /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/opt -S -passes=mem2reg,tailcallelim | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c -check-prefix=CPP-CHECK
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c -check-prefix=CPP-CHECK
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -cc1 -internal-isystem /b/1/clang-x86_64-debian-fast/llvm.obj/lib/clang/19/include -nostdsysteminc -fclang-abi-compat=latest -triple aarch64 -target-feature +sve -target-feature +bf16 -disable-O0-optnone -Werror -Wall -emit-llvm -o - -x c++ /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/opt -S -passes=mem2reg,tailcallelim
RUN: at line 5: /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -cc1 -internal-isystem /b/1/clang-x86_64-debian-fast/llvm.obj/lib/clang/19/include -nostdsysteminc -fclang-abi-compat=latest -DSVE_OVERLOADED_FORMS -triple aarch64 -target-feature +sve -target-feature +bf16 -disable-O0-optnone -Werror -Wall -emit-llvm -o - /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/opt -S -passes=mem2reg,tailcallelim | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/opt -S -passes=mem2reg,tailcallelim
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -cc1 -internal-isystem /b/1/clang-x86_64-debian-fast/llvm.obj/lib/clang/19/include -nostdsysteminc -fclang-abi-compat=latest -DSVE_OVERLOADED_FORMS -triple aarch64 -target-feature +sve -target-feature +bf16 -disable-O0-optnone -Werror -Wall -emit-llvm -o - /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
RUN: at line 6: /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -cc1 -internal-isystem /b/1/clang-x86_64-debian-fast/llvm.obj/lib/clang/19/include -nostdsysteminc -fclang-abi-compat=latest -DSVE_OVERLOADED_FORMS -triple aarch64 -target-feature +sve -target-feature +bf16 -disable-O0-optnone -Werror -Wall -emit-llvm -o - -x c++ /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/opt -S -passes=mem2reg,tailcallelim | /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c -check-prefix=CPP-CHECK
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/opt -S -passes=mem2reg,tailcallelim
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c -check-prefix=CPP-CHECK
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -cc1 -internal-isystem /b/1/clang-x86_64-debian-fast/llvm.obj/lib/clang/19/include -nostdsysteminc -fclang-abi-compat=latest -DSVE_OVERLOADED_FORMS -triple aarch64 -target-feature +sve -target-feature +bf16 -disable-O0-optnone -Werror -Wall -emit-llvm -o - -x c++ /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
RUN: at line 7: /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -cc1 -internal-isystem /b/1/clang-x86_64-debian-fast/llvm.obj/lib/clang/19/include -nostdsysteminc -fclang-abi-compat=latest -triple aarch64 -target-feature +b16 -target-feature +sve -target-feature +sme -S -disable-O0-optnone -Werror -Wall -o /dev/null /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -cc1 -internal-isystem /b/1/clang-x86_64-debian-fast/llvm.obj/lib/clang/19/include -nostdsysteminc -fclang-abi-compat=latest -triple aarch64 -target-feature +b16 -target-feature +sve -target-feature +sme -S -disable-O0-optnone -Werror -Wall -o /dev/null /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
/b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c:39:10: error: 'svcreate2_bf16' needs target feature (sve|sme),bf16
   39 |   return SVE_ACLE_FUNC(svcreate2,_bf16,,)(x0, x1);
      |          ^
/b/1/clang-x86_64-debian-fast/llvm.src/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c:16:36: note: expanded from macro 'SVE_ACLE_FUNC'
   16 | #define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
      |                                    ^
<scratch space>:9:1: note: expanded from here
    9 | svcreate2_bf16
      | ^
1 error generated.

--

********************


AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
…lvm#96220)

Fixes llvm#96197.

A global alias should always point to a definition. Ifuncs are
definitions, so far so good. However an ifunc may be statically resolved
to a function that is declared but not defined in the translation unit.

With this patch we perform static resolution if:
 * the resolvee is defined, else if
 * none of the ifunc users is a global alias
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.

[GlobalOpt] Crash when optimizing global alias to ifunc.
4 participants