Skip to content

[SYCL] Do not emit unneeded static initializations in sycl device code #1774

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 3 commits into from
Jun 8, 2020
Merged

[SYCL] Do not emit unneeded static initializations in sycl device code #1774

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96ef141
[SYCL] Do not emit unneeded static initializations in sycl device code
premanandrao May 28, 2020
016d98c
[SYCL] Do not emit unneeded static initializations in sycl device code
premanandrao Jun 4, 2020
283a81f
[SYCL] Do not emit unneeded static initializations in sycl device code
premanandrao Jun 4, 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
7 changes: 6 additions & 1 deletion clang/lib/CodeGen/CodeGenModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1276,7 +1276,12 @@ void CodeGenModule::EmitCtorList(CtorList &Fns, const char *GlobalName) {
ctor.addInt(Int32Ty, I.Priority);
ctor.add(llvm::ConstantExpr::getBitCast(I.Initializer, CtorPFTy));
if (I.AssociatedData)
ctor.add(llvm::ConstantExpr::getBitCast(I.AssociatedData, VoidPtrTy));
if (I.AssociatedData->getType()->getPointerAddressSpace() !=
VoidPtrTy->getAddressSpace())
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we check that it is correct to do address space cast here?
Like this check

llvm/clang/lib/CodeGen/CGExprScalar.cpp

Line 2996 in ccefc93

static void ensureSameAddrSpace(Value *&RHS, Value *&LHS,

@asavonic ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we have to ensure that an addrspace cast is valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, will add this check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the cases above, it looks like they values are both cast to the generic address space, to ensure sameness. This does not seem to be needed here, as the cast is to address space 0. However, I have changed the call to use an existing routine (which does the check as well) which is called in few other places, instead of checking it directly how I had it before.

ctor.add(
llvm::ConstantExpr::getAddrSpaceCast(I.AssociatedData, VoidPtrTy));
Copy link
Contributor

Choose a reason for hiding this comment

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

You are adding a new address space cast, but I don't see any address space casts in the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I will modify the test to show this.

else
ctor.add(llvm::ConstantExpr::getBitCast(I.AssociatedData, VoidPtrTy));
else
ctor.addNullPointer(VoidPtrTy);
ctor.finishAndAddTo(ctors);
Expand Down
13 changes: 9 additions & 4 deletions clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5264,16 +5264,21 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation,

// Make sure to pass the instantiated variable to the consumer at the end.
struct PassToConsumerRAII {
Sema &SemaRef;
ASTConsumer &Consumer;
VarDecl *Var;

PassToConsumerRAII(ASTConsumer &Consumer, VarDecl *Var)
: Consumer(Consumer), Var(Var) { }
PassToConsumerRAII(Sema &SemaRef, ASTConsumer &Consumer, VarDecl *Var)
: SemaRef(SemaRef), Consumer(Consumer), Var(Var) {}

~PassToConsumerRAII() {
Consumer.HandleCXXStaticMemberVarInstantiation(Var);
// Do not explicitly emit non-const static data member definitions
// on SYCL device.
if (!SemaRef.getLangOpts().SYCLIsDevice || !Var->isStaticDataMember() ||
Var->isConstexpr() || Var->getType().isConstQualified())
Consumer.HandleCXXStaticMemberVarInstantiation(Var);
}
} PassToConsumerRAII(Consumer, Var);
} PassToConsumerRAII(*this, Consumer, Var);

// If we already have a definition, we're done.
if (VarDecl *Def = Var->getDefinition()) {
Expand Down
42 changes: 42 additions & 0 deletions clang/test/CodeGenSYCL/sycl-device-static-init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// RUN: %clang_cc1 -fsycl -fsycl-is-device -triple spir64-unknown-unknown-sycldevice -disable-llvm-passes %s -emit-llvm -o - | FileCheck %s
// Test that static initializers do not force the emission of globals on sycl device

// CHECK: %struct._ZTS16RegisterBaseInit.RegisterBaseInit = type { i8 }
// CHECK-NOT: $_ZN8BaseInitI12TestBaseTypeE15s_regbase_ncsdmE = comdat any
// CHECK: $_ZN8BaseInitI12TestBaseTypeE3varE = comdat any
// CHECK: @_ZN8BaseInitI12TestBaseTypeE9s_regbaseE = {{.*}} global %struct._ZTS16RegisterBaseInit.RegisterBaseInit
// CHECK-NOT: @_ZN8BaseInitI12TestBaseTypeE15s_regbase_ncsdmE = weak_odr addrspace(1) global %struct._ZTS16RegisterBaseInit.RegisterBaseInit zeroinitializer, comdat, align 1
// CHECK: @_ZN8BaseInitI12TestBaseTypeE3varE = weak_odr addrspace(1) constant i32 9, comdat, align 4
// CHECK-NOT: @_ZGVN8BaseInitI12TestBaseTypeE15s_regbase_ncsdmE = weak_odr global i64 0, comdat($_ZN8BaseInitI12TestBaseTypeE9s_regbaseE), align 8
// CHECK: define spir_kernel void @_ZTSZ4mainE11fake_kernel()
// CHECK: call spir_func void @"_ZZ4mainENK3$_0clE16RegisterBaseInit
// CHECK: declare spir_func void @_ZN16RegisterBaseInit3fooEv

struct TestBaseType {};
struct RegisterBaseInit {
__attribute__((sycl_device)) void foo();
RegisterBaseInit();
};
template <class T>
struct BaseInit {
static const RegisterBaseInit s_regbase;
static RegisterBaseInit s_regbase_ncsdm;
static const int var;
};
template <class T>
const RegisterBaseInit BaseInit<T>::s_regbase;
template <class T>
RegisterBaseInit BaseInit<T>::s_regbase_ncsdm;
template <class T>
const int BaseInit<T>::var = 9;
template struct BaseInit<TestBaseType>;
template <typename name, typename Func>
__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
kernelFunc(BaseInit<TestBaseType>::s_regbase);
}
int main() {
kernel_single_task<class fake_kernel>([=](RegisterBaseInit s) {
s.foo();
});
return 0;
}