Skip to content

Commit

Permalink
[X86] [ABI] Fix i386 ABI "__m64" type bug
Browse files Browse the repository at this point in the history 
According to System V i386 ABI: the  __m64 type paramater and return
value are passed by MMX registers. But current implementation treats
__m64 as i64 which results in parameter passing by stack and returning
by EDX and EAX.

This patch fixes the bug (https://bugs.llvm.org/show_bug.cgi?id=41029)
for Linux and NetBSD.

Patch by Wei Xiao (wxiao3)

Differential Revision: https://reviews.llvm.org/D59744

llvm-svn: 363116
  • Loading branch information
@phoebewang
phoebewang committed Jun 12, 2019
1 parent dd4bed3 commit fbfee60
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 22 deletions.
56 changes: 36 additions & 20 deletions clang/lib/CodeGen/TargetInfo.cpp
View file
Expand Up @@ -915,14 +915,6 @@ ABIArgInfo PNaClABIInfo::classifyReturnType(QualType RetTy) const {
: ABIArgInfo::getDirect());
}

/// IsX86_MMXType - Return true if this is an MMX type.
bool IsX86_MMXType(llvm::Type *IRType) {
// Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
cast<llvm::VectorType>(IRType)->getElementType()->isIntegerTy() &&
IRType->getScalarSizeInBits() != 64;
}

static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction &CGF,
StringRef Constraint,
llvm::Type* Ty) {
Expand Down Expand Up @@ -1011,6 +1003,7 @@ class X86_32ABIInfo : public SwiftABIInfo {
bool IsSoftFloatABI;
bool IsMCUABI;
unsigned DefaultNumRegisterParameters;
bool IsMMXEnabled;

static bool isRegisterSize(unsigned Size) {
return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
Expand Down Expand Up @@ -1070,13 +1063,15 @@ class X86_32ABIInfo : public SwiftABIInfo {

X86_32ABIInfo(CodeGen::CodeGenTypes &CGT, bool DarwinVectorABI,
bool RetSmallStructInRegABI, bool Win32StructABI,
unsigned NumRegisterParameters, bool SoftFloatABI)
unsigned NumRegisterParameters, bool SoftFloatABI,
bool MMXEnabled)
: SwiftABIInfo(CGT), IsDarwinVectorABI(DarwinVectorABI),
IsRetSmallStructInRegABI(RetSmallStructInRegABI),
IsWin32StructABI(Win32StructABI),
IsSoftFloatABI(SoftFloatABI),
IsMCUABI(CGT.getTarget().getTriple().isOSIAMCU()),
DefaultNumRegisterParameters(NumRegisterParameters) {}
DefaultNumRegisterParameters(NumRegisterParameters),
IsMMXEnabled(MMXEnabled) {}

bool shouldPassIndirectlyForSwift(ArrayRef<llvm::Type*> scalars,
bool asReturnValue) const override {
Expand All @@ -1091,16 +1086,30 @@ class X86_32ABIInfo : public SwiftABIInfo {
// x86-32 lowering does not support passing swifterror in a register.
return false;
}

bool isPassInMMXRegABI() const {
// The System V i386 psABI requires __m64 to be passed in MMX registers.
// Clang historically had a bug where it failed to apply this rule, and
// some platforms (e.g. Darwin, PS4, and FreeBSD) have opted to maintain
// compatibility with the old Clang behavior, so we only apply it on
// platforms that have specifically requested it (currently just Linux and
// NetBSD).
const llvm::Triple &T = getTarget().getTriple();
if (IsMMXEnabled && (T.isOSLinux() || T.isOSNetBSD()))
return true;
return false;
}
};

class X86_32TargetCodeGenInfo : public TargetCodeGenInfo {
public:
X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, bool DarwinVectorABI,
bool RetSmallStructInRegABI, bool Win32StructABI,
unsigned NumRegisterParameters, bool SoftFloatABI)
unsigned NumRegisterParameters, bool SoftFloatABI,
bool MMXEnabled = false)
: TargetCodeGenInfo(new X86_32ABIInfo(
CGT, DarwinVectorABI, RetSmallStructInRegABI, Win32StructABI,
NumRegisterParameters, SoftFloatABI)) {}
NumRegisterParameters, SoftFloatABI, MMXEnabled)) {}

static bool isStructReturnInRegABI(
const llvm::Triple &Triple, const CodeGenOptions &Opts);
Expand Down Expand Up @@ -1386,10 +1395,9 @@ ABIArgInfo X86_32ABIInfo::classifyReturnType(QualType RetTy,
}

if (const VectorType *VT = RetTy->getAs<VectorType>()) {
uint64_t Size = getContext().getTypeSize(RetTy);
// On Darwin, some vectors are returned in registers.
if (IsDarwinVectorABI) {
uint64_t Size = getContext().getTypeSize(RetTy);

// 128-bit vectors are a special case; they are returned in
// registers and we need to make sure to pick a type the LLVM
// backend will like.
Expand All @@ -1407,6 +1415,10 @@ ABIArgInfo X86_32ABIInfo::classifyReturnType(QualType RetTy,
return getIndirectReturnResult(RetTy, State);
}

if (VT->getElementType()->isIntegerType() && Size == 64 &&
isPassInMMXRegABI())
return ABIArgInfo::getDirect(llvm::Type::getX86_MMXTy(getVMContext()));

return ABIArgInfo::getDirect();
}

Expand Down Expand Up @@ -1701,23 +1713,26 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty,
}

if (const VectorType *VT = Ty->getAs<VectorType>()) {
uint64_t Size = getContext().getTypeSize(Ty);
// On Darwin, some vectors are passed in memory, we handle this by passing
// it as an i8/i16/i32/i64.
if (IsDarwinVectorABI) {
uint64_t Size = getContext().getTypeSize(Ty);
if ((Size == 8 || Size == 16 || Size == 32) ||
(Size == 64 && VT->getNumElements() == 1))
return ABIArgInfo::getDirect(llvm::IntegerType::get(getVMContext(),
Size));
}

if (IsX86_MMXType(CGT.ConvertType(Ty)))
return ABIArgInfo::getDirect(llvm::IntegerType::get(getVMContext(), 64));

if (VT->getElementType()->isIntegerType() && Size == 64) {
if (isPassInMMXRegABI())
return ABIArgInfo::getDirect(llvm::Type::getX86_MMXTy(getVMContext()));
else
return ABIArgInfo::getDirect(
llvm::IntegerType::get(getVMContext(), 64));
}
return ABIArgInfo::getDirect();
}


if (const EnumType *EnumTy = Ty->getAs<EnumType>())
Ty = EnumTy->getDecl()->getIntegerType();

Expand Down Expand Up @@ -9460,10 +9475,11 @@ const TargetCodeGenInfo &CodeGenModule::getTargetCodeGenInfo() {
Types, IsDarwinVectorABI, RetSmallStructInRegABI,
IsWin32FloatStructABI, CodeGenOpts.NumRegisterParameters));
} else {
bool EnableMMX = getContext().getTargetInfo().getABI() != "no-mmx";
return SetCGInfo(new X86_32TargetCodeGenInfo(
Types, IsDarwinVectorABI, RetSmallStructInRegABI,
IsWin32FloatStructABI, CodeGenOpts.NumRegisterParameters,
CodeGenOpts.FloatABI == "soft"));
CodeGenOpts.FloatABI == "soft", EnableMMX));
}
}

Expand Down
4 changes: 2 additions & 2 deletions clang/test/CodeGen/x86_32-arguments-linux.c
View file
Expand Up @@ -3,7 +3,7 @@

// CHECK-LABEL: define void @f56(
// CHECK: i8 signext %a0, %struct.s56_0* byval(%struct.s56_0) align 4 %a1,
// CHECK: i64 %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
// CHECK: <1 x double> %a4, %struct.s56_2* byval(%struct.s56_2) align 4,
// CHECK: <4 x i32> %a6, %struct.s56_3* byval(%struct.s56_3) align 4,
// CHECK: <2 x double> %a8, %struct.s56_4* byval(%struct.s56_4) align 4,
Expand All @@ -12,7 +12,7 @@

// CHECK: call void (i32, ...) @f56_0(i32 1,
// CHECK: i32 %{{.*}}, %struct.s56_0* byval(%struct.s56_0) align 4 %{{[^ ]*}},
// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
// CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval(%struct.s56_2) align 4 %{{[^ ]*}},
// CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval(%struct.s56_3) align 4 %{{[^ ]*}},
// CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval(%struct.s56_4) align 4 %{{[^ ]*}},
Expand Down
29 changes: 29 additions & 0 deletions clang/test/CodeGen/x86_32-m64.c
View file
@@ -0,0 +1,29 @@
// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-netbsd -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,NETBSD
// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32

#include <mmintrin.h>
__m64 m64;
void callee(__m64 __m1, __m64 __m2);
__m64 caller(__m64 __m1, __m64 __m2)
{
// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
// LINUX: ret x86_mmx
// NETBSD-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
// NETBSD: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
// NETBSD: ret x86_mmx
// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
// DARWIN: ret i64
// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
// IAMCU: ret <1 x i64>
// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
// WIN32: ret <1 x i64>
callee(__m2, __m1);
return m64;
}

0 comments on commit fbfee60

Please sign in to comment.