From ca4b14894346948e2788277ce53e444833daa72a Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Thu, 19 Nov 2020 07:47:15 -0800 Subject: [PATCH 1/5] [SYCL][WIP] Fix address space assertion with templates In certain complex template instantiations, address space mismatches cause the compiler to assert instead of issuing errors gracefully. This fixes one such case identified in PR #2763. Signed-off-by: Premanand M Rao --- clang/lib/Sema/SemaInit.cpp | 10 +++- clang/lib/Sema/SemaSYCL.cpp | 3 + clang/lib/Sema/SemaTemplateInstantiate.cpp | 6 +- clang/test/SemaSYCL/sycl-templ-addr-space.cpp | 57 +++++++++++++++++++ 4 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 clang/test/SemaSYCL/sycl-templ-addr-space.cpp diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index ef3061ee514fd..ff6a20698172a 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -4895,9 +4895,15 @@ static void TryReferenceInitializationCore(Sema &S, Sequence.AddReferenceBindingStep(cv1T4, ValueKind == VK_RValue); ValueKind = isLValueRef ? VK_LValue : VK_XValue; // Add addr space conversion if required. - if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) { + auto T1QualsAS = T1Quals.getAddressSpace(); + if (T1QualsAS != T2Quals.getAddressSpace()) { + if (T1QualsAS == LangAS::Default) { + Sequence.SetFailed( + InitializationSequence::FK_ReferenceAddrspaceMismatchTemporary); + return; + } auto T4Quals = cv1T4.getQualifiers(); - T4Quals.addAddressSpace(T1Quals.getAddressSpace()); + T4Quals.addAddressSpace(T1QualsAS); QualType cv1T4WithAS = S.Context.getQualifiedType(T2, T4Quals); Sequence.AddQualificationConversionStep(cv1T4WithAS, ValueKind); cv1T4 = cv1T4WithAS; diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 8ee923491bff0..e72ee7862dca8 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -3314,6 +3314,9 @@ bool Sema::checkAllowedSYCLInitializer(VarDecl *VD, bool CheckValueDependent) { return true; const Expr *Init = VD->getInit(); + if (Init->containsErrors()) + return true; + bool ValueDependent = CheckValueDependent && Init->isValueDependent(); bool isConstantInit = Init && !ValueDependent && Init->isConstantInitializer(Context, false); diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 57114e4cab815..b815a66a739e1 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -670,8 +670,10 @@ void Sema::PrintInstantiationStack() { TemplateParams = cast(Active->Template) ->getTemplateParameters(); - Diags.Report(Active->PointOfInstantiation, - diag::note_prior_template_arg_substitution) + SourceLocation Loc = Active->PointOfInstantiation.isValid() ? + Active->PointOfInstantiation : + Active->Entity->getLocation(); + Diags.Report(Loc, diag::note_prior_template_arg_substitution) << isa(Parm) << Name << getTemplateArgumentBindingsText(TemplateParams, diff --git a/clang/test/SemaSYCL/sycl-templ-addr-space.cpp b/clang/test/SemaSYCL/sycl-templ-addr-space.cpp new file mode 100644 index 0000000000000..c40f7c6b2a75c --- /dev/null +++ b/clang/test/SemaSYCL/sycl-templ-addr-space.cpp @@ -0,0 +1,57 @@ +// RUN: %clang_cc1 -fsycl -fsycl-is-device -triple spir64 -verify %s + +template +struct c { + static constexpr int d = b; +}; +template +ab&& l(int); +// expected-error@+2 {{reference of type 'ah &&' cannot bind to a temporary object because of address space mismatch}} +template +auto ac(l(0)); +template +class o { + // expected-note-re@+2 {{in instantiation {{.*}} requested here}} + // expected-note-re@+1 {{while substituting prior template arguments {{.*}}}} + template )> + // expected-note@+1 {{possible target for call}} + static c p(); + // expected-error@+2 {{reference to overloaded function could not be resolved; did you mean to call it?}} + // expected-note-re@+1{{while substituting {{.*}}}} + decltype(p) i; +}; +// expected-note-re@+2 2{{in instantiation {{.*}} requested here}} +template +struct q : o { +}; +template +struct r { + typedef ab i; +}; +// expected-note@+1{{passing argument to parameter here}} +class ah { +}; +struct ai { + using i = __attribute__((opencl_global)) ah; +}; +template using am = ak; +// expected-note-re@+1 2{{candidate constructor {{.*}} not viable: {{.*}}}} +template class v { + template struct as; + // expected-note-re@+1 2{{in instantiation {{.*}} requested here}} + template struct as : am, at> {}; + // expected-note-re@+1 2{{in instantiation {{.*}} requested here}} + template using av = r>::d>; + // expected-note-re@+3 2{{in instantiation {{.*}} requested here}} + // expected-note-re@+2 2{{in instantiation {{.*}} required here}} + // expected-note-re@+1 {{candidate template ignored: {{.*}}}} + template > v(au &); +}; +struct x { + v<1> ay() { + ai::i *pi; + // expected-error@+2 {{no viable conversion from returned value of type 'ai::i' (aka '__global ah') to function return type 'v<1>'}} + // expected-note-re@+1 2{{while substituting {{.*}}}} + return pi[0]; + } +}; From 298c38cff83a5cfdb52b68e862bd407a990e086a Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Thu, 19 Nov 2020 08:07:36 -0800 Subject: [PATCH 2/5] Fix clang-format errors --- clang/lib/Sema/SemaTemplateInstantiate.cpp | 16 +++++++-------- clang/test/SemaSYCL/sycl-templ-addr-space.cpp | 20 ++++++++++--------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index b815a66a739e1..fcdb87d828383 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -670,16 +670,14 @@ void Sema::PrintInstantiationStack() { TemplateParams = cast(Active->Template) ->getTemplateParameters(); - SourceLocation Loc = Active->PointOfInstantiation.isValid() ? - Active->PointOfInstantiation : - Active->Entity->getLocation(); + SourceLocation Loc = Active->PointOfInstantiation.isValid() + ? Active->PointOfInstantiation + : Active->Entity->getLocation(); Diags.Report(Loc, diag::note_prior_template_arg_substitution) - << isa(Parm) - << Name - << getTemplateArgumentBindingsText(TemplateParams, - Active->TemplateArgs, - Active->NumTemplateArgs) - << Active->InstantiationRange; + << isa(Parm) << Name + << getTemplateArgumentBindingsText( + TemplateParams, Active->TemplateArgs, Active->NumTemplateArgs) + << Active->InstantiationRange; break; } diff --git a/clang/test/SemaSYCL/sycl-templ-addr-space.cpp b/clang/test/SemaSYCL/sycl-templ-addr-space.cpp index c40f7c6b2a75c..b25634013acaa 100644 --- a/clang/test/SemaSYCL/sycl-templ-addr-space.cpp +++ b/clang/test/SemaSYCL/sycl-templ-addr-space.cpp @@ -1,15 +1,17 @@ // RUN: %clang_cc1 -fsycl -fsycl-is-device -triple spir64 -verify %s -template -struct c { - static constexpr int d = b; +// Test that an error is issued instead of an assertion for this test case. + +template +struct c { + static constexpr int d = b; }; template -ab&& l(int); +ab &&l(int); // expected-error@+2 {{reference of type 'ah &&' cannot bind to a temporary object because of address space mismatch}} template auto ac(l(0)); -template +template class o { // expected-note-re@+2 {{in instantiation {{.*}} requested here}} // expected-note-re@+1 {{while substituting prior template arguments {{.*}}}} @@ -24,8 +26,8 @@ class o { template struct q : o { }; -template -struct r { +template +struct r { typedef ab i; }; // expected-note@+1{{passing argument to parameter here}} @@ -50,8 +52,8 @@ template class v { struct x { v<1> ay() { ai::i *pi; - // expected-error@+2 {{no viable conversion from returned value of type 'ai::i' (aka '__global ah') to function return type 'v<1>'}} - // expected-note-re@+1 2{{while substituting {{.*}}}} + // expected-error@+2 {{no viable conversion from returned value of type 'ai::i' (aka '__global ah') to function return type 'v<1>'}} + // expected-note-re@+1 2{{while substituting {{.*}}}} return pi[0]; } }; From c573be0c9db01a9276acb0484b4fc20389e56869 Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Tue, 1 Dec 2020 08:22:49 -0800 Subject: [PATCH 3/5] Address code-review comments --- clang/include/clang/AST/Type.h | 4 ++-- clang/lib/Sema/SemaInit.cpp | 12 ++++-------- clang/lib/Sema/SemaSYCL.cpp | 3 --- clang/lib/Sema/SemaTemplateInstantiate.cpp | 16 ++++++++-------- clang/test/SemaSYCL/sycl-templ-addr-space.cpp | 19 +++++++------------ 5 files changed, 21 insertions(+), 33 deletions(-) diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index 57115b10ebf57..e5696be6c6f7a 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -383,8 +383,8 @@ class Qualifiers { | (((uint32_t) space) << AddressSpaceShift); } void removeAddressSpace() { setAddressSpace(LangAS::Default); } - void addAddressSpace(LangAS space) { - assert(space != LangAS::Default); + void addAddressSpace(LangAS space, bool AllowDefaultAddrSpace = false) { + assert(space != LangAS::Default || AllowDefaultAddrSpace); setAddressSpace(space); } diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index ff6a20698172a..a5f75c90083dc 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -4895,15 +4895,11 @@ static void TryReferenceInitializationCore(Sema &S, Sequence.AddReferenceBindingStep(cv1T4, ValueKind == VK_RValue); ValueKind = isLValueRef ? VK_LValue : VK_XValue; // Add addr space conversion if required. - auto T1QualsAS = T1Quals.getAddressSpace(); - if (T1QualsAS != T2Quals.getAddressSpace()) { - if (T1QualsAS == LangAS::Default) { - Sequence.SetFailed( - InitializationSequence::FK_ReferenceAddrspaceMismatchTemporary); - return; - } + if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) { auto T4Quals = cv1T4.getQualifiers(); - T4Quals.addAddressSpace(T1QualsAS); + T4Quals.addAddressSpace( + T1Quals.getAddressSpace(), + S.getLangOpts().SYCLIsDevice); QualType cv1T4WithAS = S.Context.getQualifiedType(T2, T4Quals); Sequence.AddQualificationConversionStep(cv1T4WithAS, ValueKind); cv1T4 = cv1T4WithAS; diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index e72ee7862dca8..8ee923491bff0 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -3314,9 +3314,6 @@ bool Sema::checkAllowedSYCLInitializer(VarDecl *VD, bool CheckValueDependent) { return true; const Expr *Init = VD->getInit(); - if (Init->containsErrors()) - return true; - bool ValueDependent = CheckValueDependent && Init->isValueDependent(); bool isConstantInit = Init && !ValueDependent && Init->isConstantInitializer(Context, false); diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index fcdb87d828383..57114e4cab815 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -670,14 +670,14 @@ void Sema::PrintInstantiationStack() { TemplateParams = cast(Active->Template) ->getTemplateParameters(); - SourceLocation Loc = Active->PointOfInstantiation.isValid() - ? Active->PointOfInstantiation - : Active->Entity->getLocation(); - Diags.Report(Loc, diag::note_prior_template_arg_substitution) - << isa(Parm) << Name - << getTemplateArgumentBindingsText( - TemplateParams, Active->TemplateArgs, Active->NumTemplateArgs) - << Active->InstantiationRange; + Diags.Report(Active->PointOfInstantiation, + diag::note_prior_template_arg_substitution) + << isa(Parm) + << Name + << getTemplateArgumentBindingsText(TemplateParams, + Active->TemplateArgs, + Active->NumTemplateArgs) + << Active->InstantiationRange; break; } diff --git a/clang/test/SemaSYCL/sycl-templ-addr-space.cpp b/clang/test/SemaSYCL/sycl-templ-addr-space.cpp index b25634013acaa..c578275ca1c2e 100644 --- a/clang/test/SemaSYCL/sycl-templ-addr-space.cpp +++ b/clang/test/SemaSYCL/sycl-templ-addr-space.cpp @@ -8,21 +8,17 @@ struct c { }; template ab &&l(int); -// expected-error@+2 {{reference of type 'ah &&' cannot bind to a temporary object because of address space mismatch}} template auto ac(l(0)); template class o { - // expected-note-re@+2 {{in instantiation {{.*}} requested here}} - // expected-note-re@+1 {{while substituting prior template arguments {{.*}}}} template )> // expected-note@+1 {{possible target for call}} static c p(); - // expected-error@+2 {{reference to overloaded function could not be resolved; did you mean to call it?}} - // expected-note-re@+1{{while substituting {{.*}}}} + // expected-error@+1 {{reference to overloaded function could not be resolved; did you mean to call it?}} decltype(p) i; }; -// expected-note-re@+2 2{{in instantiation {{.*}} requested here}} +// expected-note-re@+2 {{in instantiation {{.*}} requested here}} template struct q : o { }; @@ -30,7 +26,6 @@ template struct r { typedef ab i; }; -// expected-note@+1{{passing argument to parameter here}} class ah { }; struct ai { @@ -40,12 +35,12 @@ template using am = ak; // expected-note-re@+1 2{{candidate constructor {{.*}} not viable: {{.*}}}} template class v { template struct as; - // expected-note-re@+1 2{{in instantiation {{.*}} requested here}} + // expected-note-re@+1 {{in instantiation {{.*}} requested here}} template struct as : am, at> {}; - // expected-note-re@+1 2{{in instantiation {{.*}} requested here}} + // expected-note-re@+1 {{in instantiation {{.*}} requested here}} template using av = r>::d>; - // expected-note-re@+3 2{{in instantiation {{.*}} requested here}} - // expected-note-re@+2 2{{in instantiation {{.*}} required here}} + // expected-note-re@+3 {{in instantiation {{.*}} requested here}} + // expected-note-re@+2 {{in instantiation {{.*}} required here}} // expected-note-re@+1 {{candidate template ignored: {{.*}}}} template > v(au &); }; @@ -53,7 +48,7 @@ struct x { v<1> ay() { ai::i *pi; // expected-error@+2 {{no viable conversion from returned value of type 'ai::i' (aka '__global ah') to function return type 'v<1>'}} - // expected-note-re@+1 2{{while substituting {{.*}}}} + // expected-note-re@+1 {{while substituting {{.*}}}} return pi[0]; } }; From 0d19a8a94560a41f4d600206ef84946fccbc1d55 Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Wed, 2 Dec 2020 08:20:58 -0800 Subject: [PATCH 4/5] Fix clang format errors --- clang/lib/Sema/SemaInit.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index a5f75c90083dc..01e4b6ea01917 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -4897,9 +4897,8 @@ static void TryReferenceInitializationCore(Sema &S, // Add addr space conversion if required. if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) { auto T4Quals = cv1T4.getQualifiers(); - T4Quals.addAddressSpace( - T1Quals.getAddressSpace(), - S.getLangOpts().SYCLIsDevice); + T4Quals.addAddressSpace(T1Quals.getAddressSpace(), + S.getLangOpts().SYCLIsDevice); QualType cv1T4WithAS = S.Context.getQualifiedType(T2, T4Quals); Sequence.AddQualificationConversionStep(cv1T4WithAS, ValueKind); cv1T4 = cv1T4WithAS; From 113c6b13cf23fc7e90e1025eec9d079d4ff064de Mon Sep 17 00:00:00 2001 From: Premanand M Rao Date: Thu, 3 Dec 2020 07:54:22 -0800 Subject: [PATCH 5/5] Give variables some meaningful names --- clang/test/SemaSYCL/sycl-templ-addr-space.cpp | 70 +++++++++---------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/clang/test/SemaSYCL/sycl-templ-addr-space.cpp b/clang/test/SemaSYCL/sycl-templ-addr-space.cpp index c578275ca1c2e..64e1a3659d7de 100644 --- a/clang/test/SemaSYCL/sycl-templ-addr-space.cpp +++ b/clang/test/SemaSYCL/sycl-templ-addr-space.cpp @@ -1,54 +1,52 @@ // RUN: %clang_cc1 -fsycl -fsycl-is-device -triple spir64 -verify %s -// Test that an error is issued instead of an assertion for this test case. +// Test that the compiler no longer asserts while processing this test case. -template -struct c { - static constexpr int d = b; -}; -template -ab &&l(int); -template -auto ac(l(0)); -template -class o { - template )> +template +struct integral_constant { + static constexpr int val = N; +}; +template +T &&convert(int); +template +auto declval(convert(0)); +template +class sub_group { + template )> // expected-note@+1 {{possible target for call}} - static c p(); + static integral_constant binary_op(); // expected-error@+1 {{reference to overloaded function could not be resolved; did you mean to call it?}} - decltype(p) i; + decltype(binary_op) i; }; // expected-note-re@+2 {{in instantiation {{.*}} requested here}} template -struct q : o { +struct group : sub_group { }; -template -struct r { - typedef ab i; +template +struct wrapper { + typedef T val; }; -class ah { +class element_type { }; -struct ai { - using i = __attribute__((opencl_global)) ah; +struct Container { + using __element_type = __attribute__((opencl_global)) element_type; }; -template using am = ak; +template using BaseImpl = Base; // expected-note-re@+1 2{{candidate constructor {{.*}} not viable: {{.*}}}} -template class v { - template struct as; +template class id_type { + template struct Base; // expected-note-re@+1 {{in instantiation {{.*}} requested here}} - template struct as : am, at> {}; + template struct Base : BaseImpl, Der> {}; // expected-note-re@+1 {{in instantiation {{.*}} requested here}} - template using av = r>::d>; + template using Base2 = wrapper>::val>; // expected-note-re@+3 {{in instantiation {{.*}} requested here}} // expected-note-re@+2 {{in instantiation {{.*}} required here}} // expected-note-re@+1 {{candidate template ignored: {{.*}}}} - template > v(au &); -}; -struct x { - v<1> ay() { - ai::i *pi; - // expected-error@+2 {{no viable conversion from returned value of type 'ai::i' (aka '__global ah') to function return type 'v<1>'}} - // expected-note-re@+1 {{while substituting {{.*}}}} - return pi[0]; - } -}; + template > id_type(T &); +}; +id_type<1> get() { + Container::__element_type *ElemPtr; + // expected-error@+2 {{no viable conversion from returned value of type 'Container::__element_type' (aka '__global element_type') to function return type 'id_type<1>'}} + // expected-note-re@+1 {{while substituting {{.*}}}} + return ElemPtr[0]; +}