Skip to content
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

[GlobalISel] Always direct-call IFuncs and Aliases #74902

Conversation

jroelofs
Copy link
Contributor

@jroelofs jroelofs commented Dec 9, 2023

This is safe because for both cases, the use must be in the same TU as the
definition, and they cannot be forward delcared.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 9, 2023

@llvm/pr-subscribers-llvm-globalisel

Author: Jon Roelofs (jroelofs)

Changes

This is safe because for both cases, the use must be in the same TU as the
definition, and they cannot be forward delcared.


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

3 Files Affected:

  • (modified) llvm/lib/CodeGen/GlobalISel/CallLowering.cpp (+6-1)
  • (added) llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-alias.ll (+27)
  • (added) llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll (+28)
diff --git a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
index 2527b143128967..6858e030c2c75e 100644
--- a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
@@ -146,7 +146,12 @@ bool CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, const CallBase &CB,
   const Value *CalleeV = CB.getCalledOperand()->stripPointerCasts();
   if (const Function *F = dyn_cast<Function>(CalleeV))
     Info.Callee = MachineOperand::CreateGA(F, 0);
-  else
+  else if (isa<GlobalIFunc>(CalleeV) || isa<GlobalAlias>(CalleeV)) {
+    // IR IFuncs and Aliases can't be forward declared (only defined), so the
+    // callee must be in the same TU and therefore we can direct-call it without
+    // worrying about it being out of range.
+    Info.Callee = MachineOperand::CreateGA(cast<GlobalValue>(CalleeV), 0);
+  } else
     Info.Callee = MachineOperand::CreateReg(GetCalleeReg(), false);
 
   Register ReturnHintAlignReg;
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-alias.ll b/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-alias.ll
new file mode 100644
index 00000000000000..2305c2694131ca
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-alias.ll
@@ -0,0 +1,27 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+; RUN: llc -mtriple=aarch64-elf -global-isel -stop-after=irtranslator -verify-machineinstrs -o - %s | FileCheck %s
+
+@foo_alias = alias ptr, ptr @callee
+
+define internal ptr @callee() {
+  ; CHECK-LABEL: name: callee
+  ; CHECK: bb.1.entry:
+  ; CHECK-NEXT:   [[C:%[0-9]+]]:_(p0) = G_CONSTANT i64 0
+  ; CHECK-NEXT:   $x0 = COPY [[C]](p0)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $x0
+entry:
+  ret ptr null
+}
+
+define void @caller() {
+  ; CHECK-LABEL: name: caller
+  ; CHECK: bb.1.entry:
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   BL @foo_alias, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit-def $w0
+  ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+  ; CHECK-NEXT:   RET_ReallyLR
+entry:
+  %0 = call i32 @foo_alias()
+  ret void
+}
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll b/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll
new file mode 100644
index 00000000000000..982f096fdba554
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll
@@ -0,0 +1,28 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+; RUN: llc -mtriple=aarch64-macho -global-isel -stop-after=irtranslator -verify-machineinstrs -o - %s | FileCheck %s
+; RUN: llc -mtriple=aarch64-elf -global-isel -stop-after=irtranslator -verify-machineinstrs -o - %s | FileCheck %s
+
+@foo_ifunc = ifunc i32 (i32), ptr @foo_resolver
+
+define internal ptr @foo_resolver() {
+  ; CHECK-LABEL: name: foo_resolver
+  ; CHECK: bb.1.entry:
+  ; CHECK-NEXT:   [[C:%[0-9]+]]:_(p0) = G_CONSTANT i64 0
+  ; CHECK-NEXT:   $x0 = COPY [[C]](p0)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $x0
+entry:
+  ret ptr null
+}
+
+define void @caller() {
+  ; CHECK-LABEL: name: caller
+  ; CHECK: bb.1.entry:
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   BL @foo_ifunc, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit-def $w0
+  ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+  ; CHECK-NEXT:   RET_ReallyLR
+entry:
+  %0 = call i32 @foo_ifunc()
+  ret void
+}

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 9, 2023

@llvm/pr-subscribers-backend-aarch64

Author: Jon Roelofs (jroelofs)

Changes

This is safe because for both cases, the use must be in the same TU as the
definition, and they cannot be forward delcared.


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

3 Files Affected:

  • (modified) llvm/lib/CodeGen/GlobalISel/CallLowering.cpp (+6-1)
  • (added) llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-alias.ll (+27)
  • (added) llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll (+28)
diff --git a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
index 2527b143128967..6858e030c2c75e 100644
--- a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
@@ -146,7 +146,12 @@ bool CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, const CallBase &CB,
   const Value *CalleeV = CB.getCalledOperand()->stripPointerCasts();
   if (const Function *F = dyn_cast<Function>(CalleeV))
     Info.Callee = MachineOperand::CreateGA(F, 0);
-  else
+  else if (isa<GlobalIFunc>(CalleeV) || isa<GlobalAlias>(CalleeV)) {
+    // IR IFuncs and Aliases can't be forward declared (only defined), so the
+    // callee must be in the same TU and therefore we can direct-call it without
+    // worrying about it being out of range.
+    Info.Callee = MachineOperand::CreateGA(cast<GlobalValue>(CalleeV), 0);
+  } else
     Info.Callee = MachineOperand::CreateReg(GetCalleeReg(), false);
 
   Register ReturnHintAlignReg;
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-alias.ll b/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-alias.ll
new file mode 100644
index 00000000000000..2305c2694131ca
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-alias.ll
@@ -0,0 +1,27 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+; RUN: llc -mtriple=aarch64-elf -global-isel -stop-after=irtranslator -verify-machineinstrs -o - %s | FileCheck %s
+
+@foo_alias = alias ptr, ptr @callee
+
+define internal ptr @callee() {
+  ; CHECK-LABEL: name: callee
+  ; CHECK: bb.1.entry:
+  ; CHECK-NEXT:   [[C:%[0-9]+]]:_(p0) = G_CONSTANT i64 0
+  ; CHECK-NEXT:   $x0 = COPY [[C]](p0)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $x0
+entry:
+  ret ptr null
+}
+
+define void @caller() {
+  ; CHECK-LABEL: name: caller
+  ; CHECK: bb.1.entry:
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   BL @foo_alias, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit-def $w0
+  ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+  ; CHECK-NEXT:   RET_ReallyLR
+entry:
+  %0 = call i32 @foo_alias()
+  ret void
+}
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll b/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll
new file mode 100644
index 00000000000000..982f096fdba554
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll
@@ -0,0 +1,28 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+; RUN: llc -mtriple=aarch64-macho -global-isel -stop-after=irtranslator -verify-machineinstrs -o - %s | FileCheck %s
+; RUN: llc -mtriple=aarch64-elf -global-isel -stop-after=irtranslator -verify-machineinstrs -o - %s | FileCheck %s
+
+@foo_ifunc = ifunc i32 (i32), ptr @foo_resolver
+
+define internal ptr @foo_resolver() {
+  ; CHECK-LABEL: name: foo_resolver
+  ; CHECK: bb.1.entry:
+  ; CHECK-NEXT:   [[C:%[0-9]+]]:_(p0) = G_CONSTANT i64 0
+  ; CHECK-NEXT:   $x0 = COPY [[C]](p0)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $x0
+entry:
+  ret ptr null
+}
+
+define void @caller() {
+  ; CHECK-LABEL: name: caller
+  ; CHECK: bb.1.entry:
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   BL @foo_ifunc, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit-def $w0
+  ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+  ; CHECK-NEXT:   RET_ReallyLR
+entry:
+  %0 = call i32 @foo_ifunc()
+  ret void
+}

Copy link
Contributor

@aemerson aemerson left a comment

Choose a reason for hiding this comment

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

LGTM.

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@jroelofs jroelofs changed the base branch from users/jroelofs/spr/main.globalisel-always-direct-call-ifuncs-and-aliases to main December 14, 2023 21:57
@jroelofs jroelofs merged commit b071b70 into main Dec 14, 2023
6 of 7 checks passed
@jroelofs jroelofs deleted the users/jroelofs/spr/globalisel-always-direct-call-ifuncs-and-aliases branch December 14, 2023 21:58
jroelofs added a commit that referenced this pull request Dec 15, 2023
The codegen change broke one of the BOLT tests.
jroelofs added a commit that referenced this pull request Dec 15, 2023
)

Apparently some BOLT bots build with a pre-installed system clang, and others
use the just-built one. These two clangs now behave slightly differently when
it comes to ifunc codegen after #74902

Change the test to accept both patterns.
jroelofs added a commit to apple/llvm-project that referenced this pull request Jan 10, 2024
This is safe because for both cases, the use must be in the same TU as the definition, and they cannot be forward declared.
jroelofs added a commit to apple/llvm-project that referenced this pull request Jan 10, 2024
This is safe because for both cases, the use must be in the same TU as the definition, and they cannot be forward declared.
qihangkong pushed a commit to rvgpu/rvgpu-llvm that referenced this pull request Apr 23, 2024
…902)

Apparently some BOLT bots build with a pre-installed system clang, and others
use the just-built one. These two clangs now behave slightly differently when
it comes to ifunc codegen after llvm/llvm-project#74902

Change the test to accept both patterns.
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.

None yet

3 participants