[FastISel] Don't force SDAG fallback for libcalls #171782
Merged
+6
−16
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The fast instruction selector should should not force an SDAG fallback to potentially make use of optimized libcall implementations. Note that clang will already directly emit intrinsics for most of these anyway, so there should be little change in practice.
Member
|
@llvm/pr-subscribers-backend-x86 Author: Nikita Popov (nikic) ChangesThe fast instruction selector should should not force an SDAG fallback to potentially make use of optimized libcall implementations. Looking at 3e6fa46, part of the motivation was to avoid libcalls in unoptimized builds for targets that don't have them, but I believe this should be handled by Clang directly emitting intrinsics instead of libcalls (which it already does). FastISel should not second guess this. Followup to #171288. Full diff: https://github.com/llvm/llvm-project/pull/171782.diff 3 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
index 5c84059da273b..51391f1aeecde 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1565,14 +1565,6 @@ bool FastISel::selectInstruction(const Instruction *I) {
if (const auto *Call = dyn_cast<CallInst>(I)) {
const Function *F = Call->getCalledFunction();
- LibFunc Func;
-
- // As a special case, don't handle calls to builtin library functions that
- // may be translated directly to target instructions.
- if (F && !F->hasLocalLinkage() && F->hasName() &&
- LibInfo->getLibFunc(F->getName(), Func) &&
- LibInfo->hasOptimizedCodeGen(Func))
- return false;
// Don't handle Intrinsic::trap if a trap function is specified.
if (F && F->getIntrinsicID() == Intrinsic::trap &&
diff --git a/llvm/test/CodeGen/X86/stack-protector-msvc-oz.ll b/llvm/test/CodeGen/X86/stack-protector-msvc-oz.ll
index d8a772efbd7ed..4ab23cdde74c6 100644
--- a/llvm/test/CodeGen/X86/stack-protector-msvc-oz.ll
+++ b/llvm/test/CodeGen/X86/stack-protector-msvc-oz.ll
@@ -63,11 +63,10 @@ define void @test(ptr %a) nounwind ssp minsize {
; MSVC-X86-O0-NEXT: movl ___security_cookie, %eax
; MSVC-X86-O0-NEXT: xorl %esp, %eax
; MSVC-X86-O0-NEXT: movl %eax, {{[0-9]+}}(%esp)
-; MSVC-X86-O0-NEXT: movl {{[0-9]+}}(%esp), %ecx
-; MSVC-X86-O0-NEXT: movl %esp, %eax
-; MSVC-X86-O0-NEXT: movl %ecx, 4(%eax)
+; MSVC-X86-O0-NEXT: movl {{[0-9]+}}(%esp), %eax
; MSVC-X86-O0-NEXT: leal {{[0-9]+}}(%esp), %ecx
-; MSVC-X86-O0-NEXT: movl %ecx, (%eax)
+; MSVC-X86-O0-NEXT: movl %ecx, (%esp)
+; MSVC-X86-O0-NEXT: movl %eax, {{[0-9]+}}(%esp)
; MSVC-X86-O0-NEXT: calll _strcpy
; MSVC-X86-O0-NEXT: leal LC, %ecx
; MSVC-X86-O0-NEXT: leal {{[0-9]+}}(%esp), %eax
diff --git a/llvm/test/CodeGen/X86/stack-protector-msvc.ll b/llvm/test/CodeGen/X86/stack-protector-msvc.ll
index a868fa549296d..3109733e0b0b7 100644
--- a/llvm/test/CodeGen/X86/stack-protector-msvc.ll
+++ b/llvm/test/CodeGen/X86/stack-protector-msvc.ll
@@ -75,11 +75,10 @@ define void @test(ptr %a) nounwind ssp {
; MSVC-X86-O0-NEXT: movl ___security_cookie, %eax
; MSVC-X86-O0-NEXT: xorl %esp, %eax
; MSVC-X86-O0-NEXT: movl %eax, {{[0-9]+}}(%esp)
-; MSVC-X86-O0-NEXT: movl {{[0-9]+}}(%esp), %ecx
-; MSVC-X86-O0-NEXT: movl %esp, %eax
-; MSVC-X86-O0-NEXT: movl %ecx, 4(%eax)
+; MSVC-X86-O0-NEXT: movl {{[0-9]+}}(%esp), %eax
; MSVC-X86-O0-NEXT: leal {{[0-9]+}}(%esp), %ecx
-; MSVC-X86-O0-NEXT: movl %ecx, (%eax)
+; MSVC-X86-O0-NEXT: movl %ecx, (%esp)
+; MSVC-X86-O0-NEXT: movl %eax, {{[0-9]+}}(%esp)
; MSVC-X86-O0-NEXT: calll _strcpy
; MSVC-X86-O0-NEXT: leal LC, %ecx
; MSVC-X86-O0-NEXT: leal {{[0-9]+}}(%esp), %eax
|
Member
|
@llvm/pr-subscribers-llvm-selectiondag Author: Nikita Popov (nikic) ChangesThe fast instruction selector should should not force an SDAG fallback to potentially make use of optimized libcall implementations. Looking at 3e6fa46, part of the motivation was to avoid libcalls in unoptimized builds for targets that don't have them, but I believe this should be handled by Clang directly emitting intrinsics instead of libcalls (which it already does). FastISel should not second guess this. Followup to #171288. Full diff: https://github.com/llvm/llvm-project/pull/171782.diff 3 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
index 5c84059da273b..51391f1aeecde 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1565,14 +1565,6 @@ bool FastISel::selectInstruction(const Instruction *I) {
if (const auto *Call = dyn_cast<CallInst>(I)) {
const Function *F = Call->getCalledFunction();
- LibFunc Func;
-
- // As a special case, don't handle calls to builtin library functions that
- // may be translated directly to target instructions.
- if (F && !F->hasLocalLinkage() && F->hasName() &&
- LibInfo->getLibFunc(F->getName(), Func) &&
- LibInfo->hasOptimizedCodeGen(Func))
- return false;
// Don't handle Intrinsic::trap if a trap function is specified.
if (F && F->getIntrinsicID() == Intrinsic::trap &&
diff --git a/llvm/test/CodeGen/X86/stack-protector-msvc-oz.ll b/llvm/test/CodeGen/X86/stack-protector-msvc-oz.ll
index d8a772efbd7ed..4ab23cdde74c6 100644
--- a/llvm/test/CodeGen/X86/stack-protector-msvc-oz.ll
+++ b/llvm/test/CodeGen/X86/stack-protector-msvc-oz.ll
@@ -63,11 +63,10 @@ define void @test(ptr %a) nounwind ssp minsize {
; MSVC-X86-O0-NEXT: movl ___security_cookie, %eax
; MSVC-X86-O0-NEXT: xorl %esp, %eax
; MSVC-X86-O0-NEXT: movl %eax, {{[0-9]+}}(%esp)
-; MSVC-X86-O0-NEXT: movl {{[0-9]+}}(%esp), %ecx
-; MSVC-X86-O0-NEXT: movl %esp, %eax
-; MSVC-X86-O0-NEXT: movl %ecx, 4(%eax)
+; MSVC-X86-O0-NEXT: movl {{[0-9]+}}(%esp), %eax
; MSVC-X86-O0-NEXT: leal {{[0-9]+}}(%esp), %ecx
-; MSVC-X86-O0-NEXT: movl %ecx, (%eax)
+; MSVC-X86-O0-NEXT: movl %ecx, (%esp)
+; MSVC-X86-O0-NEXT: movl %eax, {{[0-9]+}}(%esp)
; MSVC-X86-O0-NEXT: calll _strcpy
; MSVC-X86-O0-NEXT: leal LC, %ecx
; MSVC-X86-O0-NEXT: leal {{[0-9]+}}(%esp), %eax
diff --git a/llvm/test/CodeGen/X86/stack-protector-msvc.ll b/llvm/test/CodeGen/X86/stack-protector-msvc.ll
index a868fa549296d..3109733e0b0b7 100644
--- a/llvm/test/CodeGen/X86/stack-protector-msvc.ll
+++ b/llvm/test/CodeGen/X86/stack-protector-msvc.ll
@@ -75,11 +75,10 @@ define void @test(ptr %a) nounwind ssp {
; MSVC-X86-O0-NEXT: movl ___security_cookie, %eax
; MSVC-X86-O0-NEXT: xorl %esp, %eax
; MSVC-X86-O0-NEXT: movl %eax, {{[0-9]+}}(%esp)
-; MSVC-X86-O0-NEXT: movl {{[0-9]+}}(%esp), %ecx
-; MSVC-X86-O0-NEXT: movl %esp, %eax
-; MSVC-X86-O0-NEXT: movl %ecx, 4(%eax)
+; MSVC-X86-O0-NEXT: movl {{[0-9]+}}(%esp), %eax
; MSVC-X86-O0-NEXT: leal {{[0-9]+}}(%esp), %ecx
-; MSVC-X86-O0-NEXT: movl %ecx, (%eax)
+; MSVC-X86-O0-NEXT: movl %ecx, (%esp)
+; MSVC-X86-O0-NEXT: movl %eax, {{[0-9]+}}(%esp)
; MSVC-X86-O0-NEXT: calll _strcpy
; MSVC-X86-O0-NEXT: leal LC, %ecx
; MSVC-X86-O0-NEXT: leal {{[0-9]+}}(%esp), %eax
|
arsenm
approved these changes
Dec 11, 2025
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/129/builds/34828 Here is the relevant piece of the build log for the reference |
Contributor
Author
|
Huh, looks like these fallbacks did actually have significant cost: https://llvm-compile-time-tracker.com/compare.php?from=296781524902c4eb06efc5bb054cac58b973d839&to=d33d80fae606104bb82673d8a17657fe99afef70&stat=instructions:u |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The fast instruction selector should should not force an SDAG fallback to potentially make use of optimized libcall implementations.
Looking at 3e6fa46, part of the motivation was to avoid libcalls in unoptimized builds for targets that don't have them, but I believe this should be handled by Clang directly emitting intrinsics instead of libcalls (which it already does). FastISel should not second guess this.
Followup to #171288.