Anastasia added a comment.
Sam, I ran a few more tests and understood that the overhead mainly comes from
extra initialization (in Sema::Initialize()). Therefore, it's more noticeable
on a very small kernels. However, I agree we can probably neglect the overhead
as it account for only a couple % of overall time max in Release mode. Sorry,
for taking this long time.
> yaxunl wrote in DiagnosticSemaKinds.td:7998
> these two error msgs have different format.
>
> def err_type_requires_extension : Error<
> "use of type %0 requires %1 extension to be enabled">;
>
> err_type_requires_extension has an extra type name argument.
I guess we could multiplex with select and pass an empty string as a first
parameter to the diagnostic? Apart from that, the core part seems to be the
same.
> yaxunl wrote in ParsePragma.cpp:461
> Using PointerIntPair with 2 bits requires IdentifierInfo to be aligned at 4
> bytes, however this is not true. IdentifierInfo has no explicit alignment
> specifier, therefore it can only hold 1 bit in PointerIntPair.
Based on its structure with having a pointer member I think it's reasonable to
assume at least 4 bytes alignment... Although this doesn't seem to affect the
performance anyways.
> ParsePragma.cpp:1429
> PP.Lex(Tok);
> - if (Tok.isNot(tok::identifier)) {
> - PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_enable_disable);
> + if (Tok.isNot(tok::identifier) && Tok.isNot(tok::kw_register)) {
> + PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_predicate) << 0;
Not clear why register keyword is here too?
> Sema.cpp:226
> Context.getAtomicType(Context.UnsignedIntTy));
> - addImplicitTypedef("atomic_long",
> Context.getAtomicType(Context.LongTy));
> - addImplicitTypedef("atomic_ulong",
> - Context.getAtomicType(Context.UnsignedLongTy));
> + auto AtomicLongT = Context.getAtomicType(Context.LongTy);
> + addImplicitTypedef("atomic_long", AtomicLongT);
Any reason for changing this too?
> yaxunl wrote in Sema.cpp:1558
> This function can be called grammatically to set extensions for a group of
> image types, e.g., the extensions associated with image types. It is more
> convenient to allow empty string here. If empty string is not allowed, it can
> be diagnosed before calling this function.
Could we then check for an empty string ExtStr as a first function statement
instead?
> opencl-atomics-cl20.cl:51
> // expected-error@-28 {{use of type 'atomic_double' (aka '_Atomic(double)')
> requires cl_khr_int64_extended_atomics extension to be enabled}}
> -// expected-error-re@-27 {{use of type 'atomic_intptr_t' (aka
> '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be
> enabled}}
> -// expected-error-re@-28 {{use of type 'atomic_intptr_t' (aka
> '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be
> enabled}}
> -// expected-error-re@-28 {{use of type 'atomic_uintptr_t' (aka
> '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be
> enabled}}
> -// expected-error-re@-29 {{use of type 'atomic_uintptr_t' (aka
> '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be
> enabled}}
> -// expected-error-re@-29 {{use of type 'atomic_size_t' (aka
> '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be
> enabled}}
> -// expected-error-re@-30 {{use of type 'atomic_size_t' (aka
> '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be
> enabled}}
> -// expected-error-re@-30 {{use of type 'atomic_ptrdiff_t' (aka
> '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be
> enabled}}
> -// expected-error-re@-31 {{use of type 'atomic_ptrdiff_t' (aka
> '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be
> enabled}}
> +#if __LP64__
> +// expected-error-re@-28 {{use of type 'atomic_intptr_t' (aka
> '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be
> enabled}}
Why this change?
> extension-begin.cl:27
> +}
> +
> +#pragma OPENCL EXTENSION my_ext : disable
Could you please add case with typedef of a type from an extensions and also
using it with qualifiers for the better coverage.
https://reviews.llvm.org/D21698
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
