Skip to content

[X86][ABIVerifier] Verify floating point ABI correctness on 64-bit target #111690

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 43 additions & 7 deletions llvm/lib/IR/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,8 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
void verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
const Value *V, bool IsIntrinsic, bool IsInlineAsm);
void verifyFunctionMetadata(ArrayRef<std::pair<unsigned, MDNode *>> MDs);
void verifyX86ABI(FunctionType *FT, AttributeList Attrs, const Value *V,
unsigned MaxParameterWidth);

void visitConstantExprsRecursively(const Constant *EntryC);
void visitConstantExpr(const ConstantExpr *CE);
Expand Down Expand Up @@ -2086,6 +2088,45 @@ void Verifier::checkUnsignedBaseTenFuncAttr(AttributeList Attrs, StringRef Attr,
}
}

void Verifier::verifyX86ABI(FunctionType *FT, AttributeList Attrs,
const Value *V, unsigned MaxParameterWidth) {
if (!Attrs.hasFnAttr("target-features"))
return;

bool HasSSE = true;
bool HasSSE2 = true;
bool HasAVX512 = false;
bool HasEVEX512 = true;

StringRef TF = Attrs.getFnAttr("target-features").getValueAsString();
for (StringRef F : split(TF, ",")) {
StringRef S = F.substr(1);
bool Positive = F[0] == '+';
if (S == "sse")
HasSSE = Positive;
else if (S == "sse2")
HasSSE2 = Positive;
else if (S == "evex512")
HasEVEX512 = Positive;
else if (F == "-avx512f")
HasAVX512 = false;
else if (F.starts_with("+avx512"))
HasAVX512 = true;
}
// Check SSE feature.
Check(HasSSE || !TT.isArch64Bit() || !FT->getReturnType()->isFloatTy(),
"SSE register return with SSE disabled", V);
// Check SSE2 feature.
Check(HasSSE2 || !TT.isArch64Bit() ||
(!FT->getReturnType()->isDoubleTy() &&
!FT->getReturnType()->is16bitFPTy()),
"SSE2 register return with SSE2 disabled", V);
// Check EVEX512 feature.
if (MaxParameterWidth >= 512)
Check(!HasAVX512 || HasEVEX512,
"512-bit vector arguments require 'evex512' for AVX512", V);
}

// Check parameter attributes against a function type.
// The value V is printed in error messages.
void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
Expand Down Expand Up @@ -2335,13 +2376,8 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
CheckFailed("invalid value for 'frame-pointer' attribute: " + FP, V);
}

// Check EVEX512 feature.
if (MaxParameterWidth >= 512 && Attrs.hasFnAttr("target-features") &&
TT.isX86()) {
StringRef TF = Attrs.getFnAttr("target-features").getValueAsString();
Check(!TF.contains("+avx512f") || !TF.contains("-evex512"),
"512-bit vector arguments require 'evex512' for AVX512", V);
}
if (TT.isX86())
verifyX86ABI(FT, Attrs, V, MaxParameterWidth);

checkUnsignedBaseTenFuncAttr(Attrs, "patchable-function-prefix", V);
checkUnsignedBaseTenFuncAttr(Attrs, "patchable-function-entry", V);
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Analysis/CostModel/X86/arith-fp-codesize.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=code-size -mtriple=x86_64-- -mattr=-sse2 | FileCheck %s --check-prefixes=CHECK,SSE1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of dropping the test coverage - can you alter the triple to a suitable i686 target?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=code-size -mtriple=i686-- -mattr=+sse | FileCheck %s --check-prefixes=CHECK,SSE1
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=code-size -mtriple=x86_64-- -mattr=+sse2 | FileCheck %s --check-prefixes=CHECK,SSE2
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=code-size -mtriple=x86_64-- -mattr=+sse4.2 | FileCheck %s --check-prefixes=CHECK,SSE2
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=code-size -mtriple=x86_64-- -mattr=+avx | FileCheck %s --check-prefixes=CHECK,AVX
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Analysis/CostModel/X86/arith-fp-latency.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=latency -mtriple=x86_64-- -mattr=-sse2 | FileCheck %s --check-prefixes=CHECK,SSE1
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=latency -mtriple=i686-- -mattr=+sse | FileCheck %s --check-prefixes=CHECK,SSE1
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=latency -mtriple=x86_64-- -mattr=+sse2 | FileCheck %s --check-prefixes=CHECK,SSE2
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=latency -mtriple=x86_64-- -mattr=+sse4.2 | FileCheck %s --check-prefixes=CHECK,SSE42
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=latency -mtriple=x86_64-- -mattr=+avx | FileCheck %s --check-prefixes=CHECK,AVX,AVX1
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Analysis/CostModel/X86/arith-fp-sizelatency.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=size-latency -mtriple=x86_64-- -mattr=-sse2 | FileCheck %s --check-prefixes=CHECK,SSE1
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=size-latency -mtriple=i686-- -mattr=+sse | FileCheck %s --check-prefixes=CHECK,SSE1
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=size-latency -mtriple=x86_64-- -mattr=+sse2 | FileCheck %s --check-prefixes=CHECK,SSE2
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=size-latency -mtriple=x86_64-- -mattr=+sse4.2 | FileCheck %s --check-prefixes=CHECK,SSE42
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -cost-kind=size-latency -mtriple=x86_64-- -mattr=+avx | FileCheck %s --check-prefixes=CHECK,AVX
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Analysis/CostModel/X86/arith-fp.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -mtriple=x86_64-- -mattr=-sse2 | FileCheck %s --check-prefixes=SSE1
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -mtriple=i686-- -mattr=+sse | FileCheck %s --check-prefixes=SSE1
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -mtriple=x86_64-- -mattr=+sse2 | FileCheck %s --check-prefixes=SSE2
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -mtriple=x86_64-- -mattr=+sse4.2 | FileCheck %s --check-prefixes=SSE42
; RUN: opt < %s -enable-no-nans-fp-math -passes="print<cost-model>" 2>&1 -disable-output -mtriple=x86_64-- -mattr=+avx | FileCheck %s --check-prefixes=AVX,AVX1
Expand Down
17 changes: 0 additions & 17 deletions llvm/test/CodeGen/X86/GlobalISel/regbankselect-x87.ll
Original file line number Diff line number Diff line change
Expand Up @@ -138,23 +138,6 @@ define void @f3(ptr %a, ptr %b) {
ret void
}

define float @f4(float %val) {
; X86-LABEL: name: f4
; X86: bb.1 (%ir-block.0):
; X86-NEXT: [[FRAME_INDEX:%[0-9]+]]:gpr(p0) = G_FRAME_INDEX %fixed-stack.0
; X86-NEXT: [[LOAD:%[0-9]+]]:psr(s32) = G_LOAD [[FRAME_INDEX]](p0) :: (invariant load (s32) from %fixed-stack.0)
; X86-NEXT: $fp0 = COPY [[LOAD]](s32)
; X86-NEXT: RET 0, implicit $fp0
;
; X64-LABEL: name: f4
; X64: bb.1 (%ir-block.0):
; X64-NEXT: [[FRAME_INDEX:%[0-9]+]]:gpr(p0) = G_FRAME_INDEX %fixed-stack.0
; X64-NEXT: [[LOAD:%[0-9]+]]:gpr(s32) = G_LOAD [[FRAME_INDEX]](p0) :: (invariant load (s32) from %fixed-stack.0, align 16)
; X64-NEXT: $xmm0 = COPY [[LOAD]](s32)
; X64-NEXT: RET 0, implicit $xmm0
ret float %val
}

define void @f5(ptr %a, ptr %b) {
; X86-LABEL: name: f5
; X86: bb.1 (%ir-block.0):
Expand Down
Loading