-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang] Fix inconsistencies with the device_kernel attr on different targets #161905
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?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,6 +61,8 @@ class CommonSPIRTargetCodeGenInfo : public TargetCodeGenInfo { | |
QualType SampledType, CodeGenModule &CGM) const; | ||
void | ||
setOCLKernelStubCallingConvention(const FunctionType *&FT) const override; | ||
void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV, | ||
CodeGen::CodeGenModule &M) const override; | ||
}; | ||
class SPIRVTargetCodeGenInfo : public CommonSPIRTargetCodeGenInfo { | ||
public: | ||
|
@@ -240,6 +242,26 @@ void CommonSPIRTargetCodeGenInfo::setOCLKernelStubCallingConvention( | |
FT, FT->getExtInfo().withCallingConv(CC_SpirFunction)); | ||
} | ||
|
||
void CommonSPIRTargetCodeGenInfo::setTargetAttributes( | ||
const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const { | ||
if (M.getLangOpts().OpenCL) | ||
return; | ||
|
||
if (GV->isDeclaration()) | ||
return; | ||
sarnex marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
llvm::Function *F = dyn_cast<llvm::Function>(GV); | ||
if (!F) | ||
return; | ||
sarnex marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
const FunctionDecl *FD = dyn_cast<FunctionDecl>(D); | ||
if (!FD) | ||
return; | ||
|
||
if (FD->hasAttr<DeviceKernelAttr>()) | ||
F->setCallingConv(getDeviceKernelCallingConv()); | ||
} | ||
|
||
LangAS | ||
SPIRVTargetCodeGenInfo::getGlobalVarAddressSpace(CodeGenModule &CGM, | ||
const VarDecl *D) const { | ||
|
@@ -264,9 +286,6 @@ SPIRVTargetCodeGenInfo::getGlobalVarAddressSpace(CodeGenModule &CGM, | |
|
||
void SPIRVTargetCodeGenInfo::setTargetAttributes( | ||
const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const { | ||
if (!M.getLangOpts().HIP || | ||
M.getTarget().getTriple().getVendor() != llvm::Triple::AMD) | ||
return; | ||
if (GV->isDeclaration()) | ||
return; | ||
|
||
|
@@ -277,6 +296,14 @@ void SPIRVTargetCodeGenInfo::setTargetAttributes( | |
auto FD = dyn_cast_or_null<FunctionDecl>(D); | ||
if (!FD) | ||
return; | ||
|
||
if (FD->hasAttr<DeviceKernelAttr>()) | ||
F->setCallingConv(llvm::CallingConv::SPIR_KERNEL); | ||
|
||
|
||
if (!M.getLangOpts().HIP || | ||
M.getTarget().getTriple().getVendor() != llvm::Triple::AMD) | ||
return; | ||
|
||
if (!FD->hasAttr<CUDAGlobalAttr>()) | ||
return; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5204,25 +5204,55 @@ static void handleCallConvAttr(Sema &S, Decl *D, const ParsedAttr &AL) { | |
static void handleDeviceKernelAttr(Sema &S, Decl *D, const ParsedAttr &AL) { | ||
const auto *FD = dyn_cast_or_null<FunctionDecl>(D); | ||
bool IsFunctionTemplate = FD && FD->getDescribedFunctionTemplate(); | ||
if (S.getLangOpts().SYCLIsDevice) { | ||
llvm::Triple Triple = S.getASTContext().getTargetInfo().getTriple(); | ||
const LangOptions &LangOpts = S.getLangOpts(); | ||
|
||
if (LangOpts.SYCLIsDevice) { | ||
if (!IsFunctionTemplate) { | ||
S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type_str) | ||
<< AL << AL.isRegularKeywordAttribute() << "function templates"; | ||
AL.setInvalid(); | ||
return; | ||
sarnex marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} else { | ||
S.SYCL().handleKernelAttr(D, AL); | ||
} | ||
} else if (DeviceKernelAttr::isSYCLSpelling(AL)) { | ||
S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL; | ||
} else if (S.getASTContext().getTargetInfo().getTriple().isNVPTX()) { | ||
AL.setInvalid(); | ||
|
||
return; | ||
} else if (Triple.isNVPTX()) { | ||
handleGlobalAttr(S, D, AL); | ||
} else { | ||
// OpenCL C++ will throw a more specific error. | ||
if (!S.getLangOpts().OpenCLCPlusPlus && (!FD || IsFunctionTemplate)) { | ||
if (!LangOpts.OpenCLCPlusPlus && (!FD || IsFunctionTemplate)) { | ||
S.Diag(AL.getLoc(), diag::err_attribute_wrong_decl_type_str) | ||
<< AL << AL.isRegularKeywordAttribute() << "functions"; | ||
AL.setInvalid(); | ||
return; | ||
} | ||
handleSimpleAttribute<DeviceKernelAttr>(S, D, AL); | ||
} | ||
// TODO: isGPU() should probably return true for SPIR. | ||
bool TargetDeviceEnvironment = Triple.isGPU() || Triple.isSPIR() || | ||
LangOpts.isTargetDevice() || LangOpts.OpenCL; | ||
bool IsAMDGPUMismatch = | ||
DeviceKernelAttr::isAMDGPUSpelling(AL) && !Triple.isAMDGPU(); | ||
bool IsNVPTXMismatch = | ||
DeviceKernelAttr::isNVPTXSpelling(AL) && !Triple.isNVPTX(); | ||
if (IsAMDGPUMismatch || IsNVPTXMismatch || !TargetDeviceEnvironment) { | ||
Comment on lines
+5234
to
+5241
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly I'm wondering what we should do here. Realistically it would make sense for these 'named' attributes to just be legacy aliases to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would definitely be simpler to have them as aliases. I'm happy to implement whatever the consensus us, so others feel free to drop in. |
||
// While both are just different spellings of the same underlying | ||
// attribute, it makes more sense to the user if amdgpu_kernel can only | ||
// be used on AMDGPU and the equivalent for NVPTX, so warn and ignore | ||
// the attribute if there's a mismatch. | ||
// Also warn if this is not an environment where a device kernel makes | ||
// sense. | ||
S.Diag(AL.getLoc(), diag::warn_cconv_unsupported) | ||
<< AL << (int)Sema::CallingConventionIgnoredReason::ForThisTarget; | ||
AL.setInvalid(); | ||
return; | ||
} | ||
|
||
// Make sure we validate the CC with the target | ||
// and warn/error if necessary. | ||
handleCallConvAttr(S, D, AL); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -134,7 +134,6 @@ static void diagnoseBadTypeAttribute(Sema &S, const ParsedAttr &attr, | |
case ParsedAttr::AT_VectorCall: \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||
case ParsedAttr::AT_AArch64VectorPcs: \ | ||
case ParsedAttr::AT_AArch64SVEPcs: \ | ||
case ParsedAttr::AT_DeviceKernel: \ | ||
case ParsedAttr::AT_MSABI: \ | ||
case ParsedAttr::AT_SysVABI: \ | ||
case ParsedAttr::AT_Pcs: \ | ||
|
@@ -3781,7 +3780,8 @@ static CallingConv getCCForDeclaratorChunk( | |
} | ||
} | ||
if (!S.getLangOpts().isSYCL()) { | ||
for (const ParsedAttr &AL : D.getDeclSpec().getAttributes()) { | ||
for (const ParsedAttr &AL : llvm::concat<ParsedAttr>( | ||
D.getDeclSpec().getAttributes(), D.getAttributes())) { | ||
if (AL.getKind() == ParsedAttr::AT_DeviceKernel) { | ||
CC = CC_DeviceKernel; | ||
break; | ||
|
@@ -7565,8 +7565,6 @@ static Attr *getCCTypeAttr(ASTContext &Ctx, ParsedAttr &Attr) { | |
return createSimpleAttr<AArch64SVEPcsAttr>(Ctx, Attr); | ||
case ParsedAttr::AT_ArmStreaming: | ||
return createSimpleAttr<ArmStreamingAttr>(Ctx, Attr); | ||
case ParsedAttr::AT_DeviceKernel: | ||
return createSimpleAttr<DeviceKernelAttr>(Ctx, Attr); | ||
case ParsedAttr::AT_Pcs: { | ||
// The attribute may have had a fixit applied where we treated an | ||
// identifier as a string literal. The contents of the string are valid, | ||
|
@@ -8805,16 +8803,6 @@ static void HandleHLSLParamModifierAttr(TypeProcessingState &State, | |
} | ||
} | ||
|
||
static bool isMultiSubjectAttrAllowedOnType(const ParsedAttr &Attr) { | ||
// The DeviceKernel attribute is shared for many targets, and | ||
// it is only allowed to be a type attribute with the AMDGPU | ||
// spelling, so skip processing the attr as a type attr | ||
// unless it has that spelling. | ||
if (Attr.getKind() != ParsedAttr::AT_DeviceKernel) | ||
return true; | ||
return DeviceKernelAttr::isAMDGPUSpelling(Attr); | ||
} | ||
|
||
static void processTypeAttrs(TypeProcessingState &state, QualType &type, | ||
TypeAttrLocation TAL, | ||
const ParsedAttributesView &attrs, | ||
|
@@ -9068,8 +9056,6 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type, | |
break; | ||
[[fallthrough]]; | ||
FUNCTION_TYPE_ATTRS_CASELIST: | ||
if (!isMultiSubjectAttrAllowedOnType(attr)) | ||
break; | ||
|
||
attr.setUsedAsTypeAttr(); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s 2>&1 -o -| FileCheck -check-prefix=CHECK-AMDGPU %s | ||
// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda- -emit-llvm %s 2>&1 -o -| FileCheck -check-prefix=CHECK-NVPTX %s | ||
// RUN: %clang_cc1 -triple spir64 -emit-llvm %s 2>&1 -o - | FileCheck -check-prefix=CHECK-SPIR %s | ||
// RUN: %clang_cc1 -triple spirv64 -emit-llvm %s 2>&1 -o - | FileCheck -check-prefix=CHECK-SPIR %s | ||
|
||
// CHECK-AMDGPU-DAG: amdgpu_kernel void @kernel1() | ||
// CHECK-NVPTX-DAG: ptx_kernel void @kernel1() | ||
// CHECK-SPIR-DAG: spir_kernel void @kernel1() | ||
[[clang::device_kernel]] void kernel1() {} | ||
|
||
// CHECK-AMDGPU-DAG: amdgpu_kernel void @kernel2() | ||
// CHECK-NVPTX-DAG: 14:3: warning: 'clang::amdgpu_kernel' calling convention is not supported for this target | ||
// CHECK-SPIR-DAG: 14:3: warning: 'clang::amdgpu_kernel' calling convention is not supported for this target | ||
[[clang::amdgpu_kernel]] void kernel2() {} | ||
|
||
// CHECK-AMDGPU-DAG: 19:3: warning: 'clang::nvptx_kernel' calling convention is not supported for this target | ||
// CHECK-NVPTX-DAG: ptx_kernel void @kernel3() | ||
// CHECK-SPIR-DAG: 19:3: warning: 'clang::nvptx_kernel' calling convention is not supported for this target | ||
[[clang::nvptx_kernel]] void kernel3() {} | ||
|
||
// CHECK-AMDGPU-DAG: 24:3: warning: 'clang::sycl_kernel' attribute ignored | ||
// CHECK-NVPTX-DAG: 24:3: warning: 'clang::sycl_kernel' attribute ignored | ||
// CHECK-SPIR-DAG: 24:3: warning: 'clang::sycl_kernel' attribute ignored | ||
[[clang::sycl_kernel]] void kernel4() {} |
Uh oh!
There was an error while loading. Please reload this page.