Skip to content

WIP on 'visitor' model for the SemaSYCL refactor #1495

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

Closed
wants to merge 17 commits into from
Closed

WIP on 'visitor' model for the SemaSYCL refactor #1495

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9cd5f70
WIP on 'visitor' model for the SemaSYCL refactor
Apr 8, 2020
007b753
clang format
Apr 8, 2020
6f8e747
Massive set of additions/changes. Implemneted the declaration genera…
Apr 10, 2020
4d37b4c
clang-format changes
Apr 10, 2020
01da1e7
more clang-format changes
Apr 10, 2020
e5ed20c
A few spelling changes, move the kernel type changing
Apr 10, 2020
f972476
remove TODO comment, the function has now returned to a managable siz…
Apr 10, 2020
9cb607e
WIP with integration header
Apr 10, 2020
39e988c
Fix int header 'kind' calculation. Also, make sure we visit bases. …
Apr 10, 2020
055fcd4
small refactoring for base classes
Apr 10, 2020
936f36f
First run at getting the offset correct... not sure if this works
Apr 10, 2020
128f7b1
Got the offset logic compiling/looking right. Also did a clang forma…
Apr 10, 2020
645c513
Format the createOpenCLKernel func, remove int header from comment
Apr 10, 2020
7682da8
Rename kernelobj to kerneldecl
Apr 13, 2020
7735dc8
Framwork for body, added the stream type as a special type
Apr 13, 2020
a779ab2
Fix all the build errors from the last patch
Apr 13, 2020
30d446b
Add getParamVarDeclsForCurrentField, which provides a way for the bod…
Apr 13, 2020
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
240 changes: 186 additions & 54 deletions clang/lib/Sema/SemaSYCL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1256,16 +1256,6 @@ static void populateIntHeader(SYCLIntegrationHeader &H, const StringRef Name,
}
}

// Creates a mangled kernel name for given kernel name type
static std::string constructKernelName(QualType KernelNameType,
MangleContext &MC) {
SmallString<256> Result;
llvm::raw_svector_ostream Out(Result);

MC.mangleTypeName(KernelNameType, Out);
return std::string(Out.str());
}

static FunctionDecl *
CreateOpenCLKernelDeclaration(ASTContext &Context, StringRef Name,
ArrayRef<ParamDesc> ParamDescs) {
Expand Down Expand Up @@ -1307,6 +1297,153 @@ CreateOpenCLKernelDeclaration(ASTContext &Context, StringRef Name,
return OpenCLKernel;
}

// The first template argument to the kernel function is used to identify the
// kernel itself.
static QualType calculateKernelNameType(ASTContext &Ctx,
FunctionDecl *KernelCallerFunc) {
// TODO: Not sure what the 'fully qualified type's purpose is here, the type
// itself should have its full qualified name, so figure out what the purpose
// is.
const TemplateArgumentList *TAL =
KernelCallerFunc->getTemplateSpecializationArgs();
return TypeName::getFullyQualifiedType(TAL->get(0).getAsType(), Ctx,
/*WithGlobalNSPrefix=*/true);
}

// Gets a name for the kernel caller func, calculated from the first template argument.
static std::string constructKernelName(Sema &S, FunctionDecl *KernelCallerFunc,
MangleContext &MC) {
QualType KernelNameType =
calculateKernelNameType(S.getASTContext(), KernelCallerFunc);

if (S.getLangOpts().SYCLUnnamedLambda)
return PredefinedExpr::ComputeName(
S.getASTContext(), PredefinedExpr::UniqueStableNameType, KernelNameType);

SmallString<256> Result;
llvm::raw_svector_ostream Out(Result);

MC.mangleTypeName(KernelNameType, Out);
return std::string(Out.str());
}

// anonymous namespace so these don't get linkage.
namespace {
// A base type that the SYCL OpenCL Kernel construction task uses to implement
// individual tasks.
template<typename Derived>
class SyclKernelFieldHandler {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We also need one of these for the integration header generation.

protected:
Sema &SemaRef;
SyclKernelFieldHandler(Sema &S) : SemaRef(S) {}

public:
// TODO: Should the default behavior DO anything? Also, do these need any
// more params?
void handleSyclAccessorType(const FieldDecl *, QualType){}
void handleSyclSamplerType(const FieldDecl *, QualType){}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think handling for accessor and sampler types will be identical. Do we really need to split it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could swear I saw a spot where we handled them separately (but I don't have the terminal open, but I want to say the Integration Header?). I would think we can keep them split for the near future, and combine them if we find they aren't particularly useful to be separate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right. The handling in integration header can be different for accessors and samplers, but It should be identical for generation of OpenCL kernel body.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, that matches with my understanding as well! I presume the OpenCL Kernel body/decl will likely implement theirs in a function that this just calls. Maybe I'll make that part a bit easier when I get there.

void handleSyclSpecConstantType(const FieldDecl *, QualType){}
void handleStructType(const FieldDecl *, QualType){}
void handleReferenceType(const FieldDecl *, QualType){}
void handlePointerType(const FieldDecl *, QualType){}
void handleArrayType(const FieldDecl *, QualType){}
void handleScalarType(const FieldDecl *, QualType){}
};

// A type to check the valididty of all of the argument types.
class SYCLKernelFieldChecker : public SyclKernelFieldHandler<SYCLKernelFieldChecker> {
bool AllArgsValid = true;
DiagnosticsEngine &Diag;
public:
SYCLKernelFieldChecker(Sema &S)
: SyclKernelFieldHandler(S), Diag(S.getASTContext().getDiagnostics()) {}
bool isValid() { return AllArgsValid; }

void handleSyclReferenceType(const FieldDecl *FD, QualType ArgTy) {
AllArgsValid = false;
Diag.Report(FD->getLocation(), diag::err_bad_kernel_param_type) << ArgTy;
}
void handleSyclStructType(const FieldDecl *FD, QualType ArgTy) {
if (SemaRef.getASTContext().getLangOpts().SYCLStdLayoutKernelParams &&
!ArgTy->isStandardLayoutType()) {
AllArgsValid = false;
Diag.Report(FD->getLocation(), diag::err_sycl_non_std_layout_type)
<< ArgTy;
} else {
CXXRecordDecl *RD = ArgTy->getAsCXXRecordDecl();
if (!RD->hasTrivialCopyConstructor()) {
AllArgsValid = false;
Diag.Report(FD->getLocation(),
diag::err_sycl_non_trivially_copy_ctor_dtor_type)
<< 0 << ArgTy;
} else if (!RD->hasTrivialDestructor()) {
AllArgsValid = false;
Diag.Report(FD->getLocation(),
diag::err_sycl_non_trivially_copy_ctor_dtor_type)
<< 1 << ArgTy;
}
}
}
};

// A type to Create and own the FunctionDecl for the kernel.
class SYCLKernelHeader : public SyclKernelFieldHandler<SYCLKernelHeader> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this class is needed at all?
What is kernel obj header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure yet :) I was brainstorming based on the current things we do. The idea was to have this + the error-checking version replace BuildArgTys. This is more just to build/own the declaration and much of the stuff that happens in ConstructOpenCLKernel.

SYCLKernelFieldChecker &ArgChecker;
FunctionDecl *KernelObj = nullptr;

public:
// TODO: More params necessary for the kernel obj header.
SYCLKernelHeader(Sema &S, SYCLKernelFieldChecker &ArgChecker)
: SyclKernelFieldHandler(S), ArgChecker(ArgChecker) {}
~SYCLKernelHeader() {
// TODO: Should 'body' do this? Does it need to do stuff in its destructor?
if (KernelObj && ArgChecker.isValid())
SemaRef.addSyclDeviceDecl(KernelObj);
}
};

class SYCLKernelBody : public SyclKernelFieldHandler<SYCLKernelBody> {
SYCLKernelHeader &Header;

public:
SYCLKernelBody(Sema &S, SYCLKernelHeader &H)
: SyclKernelFieldHandler(S), Header(H) {}
};
}

template<typename ... Handlers>
void VisitKernelFields(RecordDecl::field_range Fields, Handlers& ... handlers) {

// Implements the 'for-each-visitor' pattern.
#define KF_FOR_EACH(FUNC) \
(void)std::initializer_list<int>{(handlers.FUNC(Field, ArgTy), 0)...}

for (const auto &Field : Fields){
QualType ArgTy = Field->getType();

if (Util::isSyclAccessorType(ArgTy))
KF_FOR_EACH(handleSyclAccessorType);
else if (Util::isSyclSamplerType(ArgTy))
KF_FOR_EACH(handleSyclSamplerType);
else if (Util::isSyclSpecConstantType(ArgTy))
KF_FOR_EACH(handleSyclSpecConstantType);
else if (ArgTy->isStructureOrClassType())
KF_FOR_EACH(handleStructType);
else if (ArgTy->isReferenceType())
KF_FOR_EACH(handleReferenceType);
else if (ArgTy->isPointerType())
KF_FOR_EACH(handlePointerType);
else if (ArgTy->isArrayType())
KF_FOR_EACH(handleArrayType);
else if (ArgTy->isScalarType())
KF_FOR_EACH(handleScalarType);
else
llvm_unreachable("Unsupported kernel parameter type");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This almost definitely shouldn't be here. I should add a "handleOtherType" that is only implemented by the diagnostics, which simply diagnoses with an error. Everyone else can ignore it.

}
#undef KF_FOR_EACH
}


// Generates the OpenCL kernel using KernelCallerFunc (kernel caller
// function) defined is SYCL headers.
// Generated OpenCL kernel contains the body of the kernel caller function,
Expand All @@ -1331,52 +1468,47 @@ CreateOpenCLKernelDeclaration(ASTContext &Context, StringRef Name,
//
void Sema::ConstructOpenCLKernel(FunctionDecl *KernelCallerFunc,
MangleContext &MC) {
CXXRecordDecl *LE = getKernelObjectType(KernelCallerFunc);
assert(LE && "invalid kernel caller");
// The first argument to the KernelCallerFunc is the lambda object.
CXXRecordDecl *KernelLambda = getKernelObjectType(KernelCallerFunc);
assert(KernelLambda && "invalid kernel caller");
std::string KernelName = constructKernelName(*this, KernelCallerFunc, MC);

// Build list of kernel arguments
llvm::SmallVector<ParamDesc, 16> ParamDescs;
if (!buildArgTys(getASTContext(), LE, ParamDescs))
return;
SYCLKernelFieldChecker checker(*this);
SYCLKernelHeader header(*this, checker);
SYCLKernelBody body(*this, header);

// Extract name from kernel caller parameters and mangle it.
const TemplateArgumentList *TemplateArgs =
KernelCallerFunc->getTemplateSpecializationArgs();
assert(TemplateArgs && "No template argument info");
QualType KernelNameType = TypeName::getFullyQualifiedType(
TemplateArgs->get(0).getAsType(), getASTContext(), true);

std::string Name;
// TODO SYCLIntegrationHeader also computes a unique stable name. It should
// probably lose this responsibility and only use the name provided here.
if (getLangOpts().SYCLUnnamedLambda)
Name = PredefinedExpr::ComputeName(
getASTContext(), PredefinedExpr::UniqueStableNameExpr, KernelNameType);
else
Name = constructKernelName(KernelNameType, MC);

// TODO Maybe don't emit integration header inside the Sema?
populateIntHeader(getSyclIntegrationHeader(), Name, KernelNameType, LE);

FunctionDecl *OpenCLKernel =
CreateOpenCLKernelDeclaration(getASTContext(), Name, ParamDescs);

ContextRAII FuncContext(*this, OpenCLKernel);

// Let's copy source location of a functor/lambda to emit nicer diagnostics
OpenCLKernel->setLocation(LE->getLocation());

// If the source function is implicitly inline, the kernel should be marked
// such as well. This allows the kernel to be ODR'd if there are multiple uses
// in different translation units.
OpenCLKernel->setImplicitlyInline(KernelCallerFunc->isInlined());

ConstructingOpenCLKernel = true;
CompoundStmt *OpenCLKernelBody =
CreateOpenCLKernelBody(*this, KernelCallerFunc, OpenCLKernel);
ConstructingOpenCLKernel = false;
OpenCLKernel->setBody(OpenCLKernelBody);
addSyclDeviceDecl(OpenCLKernel);
VisitKernelFields(KernelLambda->fields(), checker, header, body);

/*
// Build list of kernel arguments
llvm::SmallVector<ParamDesc, 16> ParamDescs;
if (!buildArgTys(getASTContext(), LE, ParamDescs))
return;

// TODO Maybe don't emit integration header inside the Sema?
populateIntHeader(getSyclIntegrationHeader(), Name, KernelNameType, LE);

FunctionDecl *OpenCLKernel =
CreateOpenCLKernelDeclaration(getASTContext(), Name, ParamDescs);

ContextRAII FuncContext(*this, OpenCLKernel);

// Let's copy source location of a functor/lambda to emit nicer diagnostics
OpenCLKernel->setLocation(LE->getLocation());

// If the source function is implicitly inline, the kernel should be marked
// such as well. This allows the kernel to be ODR'd if there are multiple
uses
// in different translation units.
OpenCLKernel->setImplicitlyInline(KernelCallerFunc->isInlined());

ConstructingOpenCLKernel = true;
CompoundStmt *OpenCLKernelBody =
CreateOpenCLKernelBody(*this, KernelCallerFunc, OpenCLKernel);
ConstructingOpenCLKernel = false;
OpenCLKernel->setBody(OpenCLKernelBody);
addSyclDeviceDecl(OpenCLKernel);
*/
}

void Sema::MarkDevice(void) {
Expand Down