Skip to content

Commit 408bad1

Browse files
committed
Updates to address code review feedback from Erich Keane.
- Changed the style of added FIXME comments. - Changed a `dyn_cast` followed by `assert` to `cast`. - Removed some unused `FileCheck` captures in a test.
1 parent 179b25e commit 408bad1

File tree

4 files changed

+14
-14
lines changed

4 files changed

+14
-14
lines changed

clang/lib/AST/ASTContext.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12805,7 +12805,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
1280512805
return true;
1280612806

1280712807
// FIXME: Functions declared with SYCL_EXTERNAL are required during
12808-
// FIXME: device compilation.
12808+
// device compilation.
1280912809

1281012810
// Constructors and destructors are required.
1281112811
if (FD->hasAttr<ConstructorAttr>() || FD->hasAttr<DestructorAttr>())
@@ -14833,10 +14833,10 @@ static SYCLKernelInfo BuildSYCLKernelInfo(ASTContext &Context,
1483314833

1483414834
// Construct a mangled name for the SYCL kernel caller offload entry point.
1483514835
// FIXME: The Itanium typeinfo mangling (_ZTS<type>) is currently used to
14836-
// FIXME: name the SYCL kernel caller offload entry point function. This
14837-
// FIXME: mangling does not suffice to clearly identify symbols that
14838-
// FIXME: correspond to SYCL kernel caller functions, nor is this mangling
14839-
// FIXME: natural for targets that use a non-Itanium ABI.
14836+
// name the SYCL kernel caller offload entry point function. This mangling
14837+
// does not suffice to clearly identify symbols that correspond to SYCL
14838+
// kernel caller functions, nor is this mangling natural for targets that
14839+
// use a non-Itanium ABI.
1484014840
std::string Buffer;
1484114841
Buffer.reserve(128);
1484214842
llvm::raw_string_ostream Out(Buffer);

clang/lib/CodeGen/CGCall.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,8 @@ CodeGenTypes::arrangeSYCLKernelCallerDeclaration(QualType resultType,
746746

747747
return arrangeLLVMFunctionInfo(
748748
GetReturnType(resultType), FnInfoOpts::None, argTypes,
749-
FunctionType::ExtInfo(CC_OpenCLKernel), {}, RequiredArgs::All);
749+
FunctionType::ExtInfo(CC_OpenCLKernel), /*paramInfos=*/{},
750+
RequiredArgs::All);
750751
}
751752

752753
/// Arrange a call to a C++ method, passing the given arguments.

clang/lib/CodeGen/CodeGenSYCL.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ void CodeGenModule::EmitSYCLKernelCaller(const FunctionDecl *KernelEntryPointFn,
3939

4040
// Find the SYCLKernelCallStmt.
4141
SYCLKernelCallStmt *KernelCallStmt =
42-
dyn_cast<SYCLKernelCallStmt>(KernelEntryPointFn->getBody());
43-
assert(KernelCallStmt && "SYCLKernelCallStmt not found");
42+
cast<SYCLKernelCallStmt>(KernelEntryPointFn->getBody());
4443

4544
// Retrieve the SYCL kernel caller parameters from the OutlinedFunctionDecl.
4645
FunctionArgList Args;

clang/test/CodeGenSYCL/kernel-caller-entry-point.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,29 +62,29 @@ int main() {
6262
// attributed functions during host compilation. ODR-use of these functions may
6363
// require them to be emitted, but they have no effect if called.
6464
//
65-
// CHECK-HOST-LINUX: define dso_local void @_Z26single_purpose_kernel_task21single_purpose_kernel() #[[LINUX_ATTR0:[0-9]+]] {
65+
// CHECK-HOST-LINUX: define dso_local void @_Z26single_purpose_kernel_task21single_purpose_kernel() #{{[0-9]+}} {
6666
// CHECK-HOST-LINUX-NEXT: entry:
6767
// CHECK-HOST-LINUX-NEXT: %kernelFunc = alloca %struct.single_purpose_kernel, align 1
6868
// CHECK-HOST-LINUX-NEXT: ret void
6969
// CHECK-HOST-LINUX-NEXT: }
7070
//
71-
// CHECK-HOST-LINUX: define internal void @_Z18kernel_single_taskIZ4mainE18lambda_kernel_nameZ4mainEUlvE_EvT0_(i32 %kernelFunc.coerce) #[[LINUX_ATTR0:[0-9]+]] {
71+
// CHECK-HOST-LINUX: define internal void @_Z18kernel_single_taskIZ4mainE18lambda_kernel_nameZ4mainEUlvE_EvT0_(i32 %kernelFunc.coerce) #{{[0-9]+}} {
7272
// CHECK-HOST-LINUX-NEXT: entry:
7373
// CHECK-HOST-LINUX-NEXT: %kernelFunc = alloca %class.anon, align 4
7474
// CHECK-HOST-LINUX-NEXT: %coerce.dive = getelementptr inbounds nuw %class.anon, ptr %kernelFunc, i32 0, i32 0
7575
// CHECK-HOST-LINUX-NEXT: store i32 %kernelFunc.coerce, ptr %coerce.dive, align 4
7676
// CHECK-HOST-LINUX-NEXT: ret void
7777
// CHECK-HOST-LINUX-NEXT: }
7878
//
79-
// CHECK-HOST-WINDOWS: define dso_local void @"?single_purpose_kernel_task@@YAXUsingle_purpose_kernel@@@Z"(i8 %kernelFunc.coerce) #[[WINDOWS_ATTR0:[0-9]+]] {
79+
// CHECK-HOST-WINDOWS: define dso_local void @"?single_purpose_kernel_task@@YAXUsingle_purpose_kernel@@@Z"(i8 %kernelFunc.coerce) #{{[0-9]+}} {
8080
// CHECK-HOST-WINDOWS-NEXT: entry:
8181
// CHECK-HOST-WINDOWS-NEXT: %kernelFunc = alloca %struct.single_purpose_kernel, align 1
8282
// CHECK-HOST-WINDOWS-NEXT: %coerce.dive = getelementptr inbounds nuw %struct.single_purpose_kernel, ptr %kernelFunc, i32 0, i32 0
8383
// CHECK-HOST-WINDOWS-NEXT: store i8 %kernelFunc.coerce, ptr %coerce.dive, align 1
8484
// CHECK-HOST-WINDOWS-NEXT: ret void
8585
// CHECK-HOST-WINDOWS-NEXT: }
8686
//
87-
// CHECK-HOST-WINDOWS: define internal void @"??$kernel_single_task@Vlambda_kernel_name@?1??main@@9@V<lambda_1>@?0??2@9@@@YAXV<lambda_1>@?0??main@@9@@Z"(i32 %kernelFunc.coerce) #[[WINDOWS_ATTR0:[0-9]+]] {
87+
// CHECK-HOST-WINDOWS: define internal void @"??$kernel_single_task@Vlambda_kernel_name@?1??main@@9@V<lambda_1>@?0??2@9@@@YAXV<lambda_1>@?0??main@@9@@Z"(i32 %kernelFunc.coerce) #{{[0-9]+}} {
8888
// CHECK-HOST-WINDOWS-NEXT: entry:
8989
// CHECK-HOST-WINDOWS-NEXT: %kernelFunc = alloca %class.anon, align 4
9090
// CHECK-HOST-WINDOWS-NEXT: %coerce.dive = getelementptr inbounds nuw %class.anon, ptr %kernelFunc, i32 0, i32 0
@@ -95,8 +95,8 @@ int main() {
9595
// Verify that SYCL kernel caller functions are emitted for each device target.
9696
//
9797
// FIXME: The following set of matches are used to skip over the declaration of
98-
// FIXME: main(). main() shouldn't be emitted in device code, but that pruning
99-
// FIXME: isn't performed yet.
98+
// main(). main() shouldn't be emitted in device code, but that pruning isn't
99+
// performed yet.
100100
// CHECK-DEVICE: Function Attrs: convergent mustprogress noinline norecurse nounwind optnone
101101
// CHECK-DEVICE-NEXT: define {{[a-z_ ]*}}noundef i32 @main() #0
102102

0 commit comments

Comments
 (0)