Skip to content

[OpenCL] Disable vector to scalar types coercion for OpenCL #8160

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Feb 20, 2023
Merged

[OpenCL] Disable vector to scalar types coercion for OpenCL #8160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
72b8863
[OpenCL] Disable vector to scalar types coercion for OpenCL
cdai2 Jan 31, 2023
a04b937
FIx clang comment punctuation.
cdai2 Feb 6, 2023
30ecfa4
add x86_64-pc-win32 test, add fixme, change 32bit triple to i686-pc-w…
wenju-he Feb 15, 2023
d322d4a
Merge branch 'sycl' into Disable_vector_to_scalar_types_coercion
bader Feb 18, 2023
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
1 change: 1 addition & 0 deletions clang/include/clang/Basic/LangOptions.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ LANGOPT(ObjCDisableDirectMethodsForTesting, 1, 0,
"Disable recognition of objc_direct methods")
LANGOPT(CFProtectionBranch , 1, 0, "Control-Flow Branch Protection enabled")
LANGOPT(FakeAddressSpaceMap , 1, 0, "OpenCL fake address space map")
LANGOPT(OpenCLForceVectorABI, 1, 0, "OpenCL vector to scalar coercion disabling")
ENUM_LANGOPT(AddressSpaceMapMangling , AddrSpaceMapMangling, 2, ASMM_Target, "OpenCL address space map mangling mode")
LANGOPT(IncludeDefaultHeader, 1, 0, "Include default header file for OpenCL")
LANGOPT(DeclareOpenCLBuiltins, 1, 0, "Declare OpenCL builtin functions")
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Driver/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6468,6 +6468,9 @@ defm const_strings : BoolOption<"f", "const-strings",
def fno_bitfield_type_align : Flag<["-"], "fno-bitfield-type-align">,
HelpText<"Ignore bit-field types when aligning structures">,
MarshallingInfoFlag<LangOpts<"NoBitFieldTypeAlign">>;
def fopencl_force_vector_abi : Flag<["-"], "fopencl-force-vector-abi">,
HelpText<"Disable vector to scalar coercion for OpenCL">,
MarshallingInfoFlag<LangOpts<"OpenCLForceVectorABI">>;
def ffake_address_space_map : Flag<["-"], "ffake-address-space-map">,
HelpText<"Use a fake address space map; OpenCL testing purposes only">,
MarshallingInfoFlag<LangOpts<"FakeAddressSpaceMap">>;
Expand Down
46 changes: 46 additions & 0 deletions clang/lib/CodeGen/TargetInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,41 @@ Address ABIInfo::EmitMSVAArg(CodeGenFunction &CGF, Address VAListAddr,
return Address::invalid();
}

static ABIArgInfo classifyOpenCL(QualType Ty, ASTContext &Context) {
if (Ty->isVoidType())
return ABIArgInfo::getIgnore();

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

if (const RecordType *RT = Ty->getAs<RecordType>())
return ABIArgInfo::getIndirect(Context.getTypeAlignInChars(RT),
/*ByVal=*/false);

if (Context.isPromotableIntegerType(Ty))
return ABIArgInfo::getExtend(Ty);

return ABIArgInfo::getDirect();
}

static bool doOpenCLClassification(CGFunctionInfo &FI, ASTContext &Context) {
if (!Context.getLangOpts().OpenCL)
return false;
if (!Context.getLangOpts().OpenCLForceVectorABI)
return false;

// Use OpenCL classify to prevent coercing.
// Vector ABI must be enforced by enabling the corresponding option.
// Otherwise, vector types will be coerced to a matching integer
// type to conform with ABI, e.g.: <8 x i8> will be coerced to i64.
FI.getReturnInfo() = classifyOpenCL(FI.getReturnType(), Context);

for (auto &Arg : FI.arguments())
Arg.info = classifyOpenCL(Arg.type, Context);

return true;
}

static llvm::Type *getVAListElementType(CodeGenFunction &CGF) {
return CGF.ConvertTypeForMem(
CGF.getContext().getBuiltinVaListType()->getPointeeType());
Expand Down Expand Up @@ -1984,6 +2019,10 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty,
}

void X86_32ABIInfo::computeInfo(CGFunctionInfo &FI) const {
ASTContext &Context = getContext();
if (doOpenCLClassification(FI, Context))
return;

CCState State(FI);
if (IsMCUABI)
State.FreeRegs = 3;
Expand Down Expand Up @@ -3970,6 +4009,9 @@ X86_64ABIInfo::classifyRegCallStructType(QualType Ty, unsigned &NeededInt,
}

void X86_64ABIInfo::computeInfo(CGFunctionInfo &FI) const {
ASTContext &Context = getContext();
if (doOpenCLClassification(FI, Context))
return;

const unsigned CallingConv = FI.getCallingConvention();
// It is possible to force Win64 calling convention on any x86_64 target by
Expand Down Expand Up @@ -4427,6 +4469,10 @@ ABIArgInfo WinX86_64ABIInfo::classify(QualType Ty, unsigned &FreeSSERegs,
}

void WinX86_64ABIInfo::computeInfo(CGFunctionInfo &FI) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you extend the test below for this target as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

triple x86_64-unknown-unknown in the test is already covering WinX86_64ABIInfo

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that because the test is being run on Windows? Otherwise, I am not sure how it defaults to Windows and not say, for example, Linux.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that because the test is being run on Windows?

Yes. On linux, the test runs in X86_64ABIInfo::computeInfo path.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, please add a Windows-specific triple to the test.

Copy link
Contributor

Choose a reason for hiding this comment

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

triple x86_64-unknown-unknown in the test is already covering WinX86_64ABIInfo

sorry this answer is incorrect. You're right that windows x86_64 target isn't covered in the first commit. I've added WinX86_64ABIInfo test in the new commit.

I also changed i686-unknown-unknown triple to i686-pc-win32-gnu, in order to match with the use scenario of this PR in OpenCL builtin build. DEVICE_TRIPLE is i686-pc-win32-gnu-elf and x86_64-pc-win32-gnu-elf for windows build in file backend/libraries/CMakeLists.txt

ASTContext &Context = getContext();
if (doOpenCLClassification(FI, Context))
return;

const unsigned CC = FI.getCallingConvention();
bool IsVectorCall = CC == llvm::CallingConv::X86_VectorCall;
bool IsRegCall = CC == llvm::CallingConv::X86_RegCall;
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3928,6 +3928,8 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
}
}

Opts.OpenCLForceVectorABI = Args.hasArg(OPT_fopencl_force_vector_abi);

// Check if -fopenmp is specified and set default version to 5.0.
Opts.OpenMP = Args.hasArg(OPT_fopenmp) ? 50 : 0;
// Check if -fopenmp-simd is specified.
Expand Down
29 changes: 29 additions & 0 deletions clang/test/CodeGenOpenCL/vector-to-scalar-coercion.cl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// RUN: %clang_cc1 -x cl -triple i686-pc-win32-gnu -fopencl-force-vector-abi %s -O0 -emit-llvm -o - | FileCheck %s --check-prefix NOCOER
// RUN: %clang_cc1 -x cl -triple x86_64-unknown-linux -fopencl-force-vector-abi %s -O0 -emit-llvm -o - | FileCheck %s --check-prefix NOCOER
// RUN: %clang_cc1 -x cl -triple x86_64-pc-win32-gnu -fopencl-force-vector-abi %s -O0 -emit-llvm -o - | FileCheck %s --check-prefix NOCOER

// RUN: %clang_cc1 -x cl -triple i686-pc-win32-gnu %s -O0 -emit-llvm -o - | FileCheck %s --check-prefix COER32CL
// RUN: %clang_cc1 -x cl -triple x86_64-unknown-linux %s -O0 -emit-llvm -o - | FileCheck %s --check-prefix COER64
// RUN: %clang_cc1 -x cl -triple x86_64-pc-win32-gnu %s -O0 -emit-llvm -o - | FileCheck %s --check-prefix NOCOER

// RUN: %clang_cc1 -x c -triple i686-pc-win32-gnu %s -O0 -emit-llvm -o - | FileCheck %s --check-prefix COER32
// RUN: %clang_cc1 -x c -triple x86_64-unknown-linux %s -O0 -emit-llvm -o - | FileCheck %s --check-prefix COER64
// RUN: %clang_cc1 -x c -triple x86_64-pc-win32-gnu %s -O0 -emit-llvm -o - | FileCheck %s --check-prefix NOCOER-C-WIN

typedef unsigned short ushort;
typedef ushort ushort4 __attribute__((ext_vector_type(4)));

typedef unsigned long ulong;
typedef ulong ulong4 __attribute__((ext_vector_type(4)));

ulong4 __attribute__((const)) __attribute__((overloadable)) convert_ulong4_rte(ushort4 x)
{
return 1;
}

// NOCOER: define {{.*}}<4 x i64> @_Z18convert_ulong4_rteDv4_t(<4 x i16> noundef %{{.*}})
// NOCOER-C-WIN: define {{.*}}<4 x i32> @_Z18convert_ulong4_rteDv4_t(<4 x i16> noundef %{{.*}})
// COER32CL: define {{.*}}<4 x i64> @_Z18convert_ulong4_rteDv4_t(i64 noundef %{{.*}})
// COER32: define {{.*}}<4 x i32> @_Z18convert_ulong4_rteDv4_t(i64 noundef %{{.*}})
// FIXME: <4 x i16> should be coerced to i64 instead of double
// COER64: define {{.*}}<4 x i64> @_Z18convert_ulong4_rteDv4_t(double noundef %{{.*}})