-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[Win][X86]Fix issue where _fltused reference is incorrectly issued for vector floating point operations #146792
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-backend-x86 Author: Adam Glass (AdamGlass) ChangesFixes #146428 _fltused reference generated for vector floating point operations Currently references to _fltused are incorrectly emitted due to the presence of vector floating point operations. This causes spurious references to _fltused in vector floating point system code where the CRT is not used. This issue is limited to the X86 MSVC environment. As described in the bug:
Have confirmed intended behavior with MSVC team. Fix alters usesMSVCFloatingPoint() to look for floating point instructions/operands rather than floating point or vector floating point. Full diff: https://github.com/llvm/llvm-project/pull/146792.diff 2 Files Affected:
diff --git a/llvm/lib/Target/X86/X86AsmPrinter.cpp b/llvm/lib/Target/X86/X86AsmPrinter.cpp
index c7238839c26b5..57a1bebb562ed 100644
--- a/llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ b/llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -1002,11 +1002,11 @@ static bool usesMSVCFloatingPoint(const Triple &TT, const Module &M) {
for (const Function &F : M) {
for (const Instruction &I : instructions(F)) {
- if (I.getType()->isFPOrFPVectorTy())
+ if (I.getType()->isFloatingPointTy())
return true;
for (const auto &Op : I.operands()) {
- if (Op->getType()->isFPOrFPVectorTy())
+ if (Op->getType()->isFloatingPointTy())
return true;
}
}
diff --git a/llvm/test/CodeGen/X86/fltused_vec.ll b/llvm/test/CodeGen/X86/fltused_vec.ll
new file mode 100644
index 0000000000000..312f71203dde8
--- /dev/null
+++ b/llvm/test/CodeGen/X86/fltused_vec.ll
@@ -0,0 +1,32 @@
+; The purpose of this test to verify that the fltused symbol is
+; not emitted when purely vector floating point operations are used on Windows.
+
+; RUN: llc < %s -mtriple i686-pc-win32 | FileCheck %s --check-prefix WIN32
+; RUN: llc < %s -mtriple x86_64-pc-win32 | FileCheck %s --check-prefix WIN64
+; RUN: llc < %s -O0 -mtriple i686-pc-win32 | FileCheck %s --check-prefix WIN32
+; RUN: llc < %s -O0 -mtriple x86_64-pc-win32 | FileCheck %s --check-prefix WIN64
+
+@foo = external dso_local global [4 x float], align 16
+
+; Function Attrs: noinline nounwind optnone sspstrong uwtable
+define dso_local <4 x float> @func() #0 {
+entry:
+ %__p.addr.i = alloca ptr, align 8
+ %vector1 = alloca <4 x float>, align 16
+ store ptr @foo, ptr %__p.addr.i, align 8
+ %0 = load ptr, ptr %__p.addr.i, align 8
+ %1 = load <4 x float>, ptr %0, align 16
+ store <4 x float> %1, ptr %vector1, align 16
+ %2 = load <4 x float>, ptr %vector1, align 16
+ ret <4 x float> %2
+}
+
+define <4 x float> @mul_vectors(<4 x float> %a, <4 x float> %b) {
+entry:
+ %result = fmul <4 x float> %a, %b
+ ret <4 x float> %result
+}
+
+; _fltused is determined at a module level
+; WIN32-NOT: .globl __fltused
+; WIN64-NOT: .globl _fltused
|
llvm/test/CodeGen/X86/fltused_vec.ll
Outdated
; RUN: llc < %s -O0 -mtriple i686-pc-win32 | FileCheck %s --check-prefix WIN32 | ||
; RUN: llc < %s -O0 -mtriple x86_64-pc-win32 | FileCheck %s --check-prefix WIN64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to test O0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Carry over from other test.
llvm/test/CodeGen/X86/fltused_vec.ll
Outdated
; WIN32-NOT: .globl __fltused | ||
; WIN64-NOT: .globl _fltused |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about use CHECK-NOT: .globl {{.*}}_fltused
. We don't need to distiguish 32/64 then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, though i used {{_?}}_fltused instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Fixes #146428 _fltused reference generated for vector floating point operations
Currently references to _fltused are incorrectly emitted due to the presence of vector floating point operations. This causes spurious references to _fltused in vector floating point system code where the CRT is not used. This issue is limited to the X86 MSVC environment.
As described in the bug:
Have confirmed intended behavior with MSVC team.
Fix alters usesMSVCFloatingPoint() to look for floating point instructions/operands rather than floating point or vector floating point.
[email protected]