Page MenuHomePhabricator

Anastasia (Anastasia Stulova)
Compiler Engineer at ARM / Code Owner of OpenCL in Clang

Projects

User does not belong to any projects.

User Details

User Since
Mar 5 2015, 8:05 AM (306 w, 2 d)

Recent Activity

Yesterday

Anastasia committed rGbc84f89c71ab: [OpenCL][Docs] Fixed cross-section reference in OpenCLSupport (authored by Anastasia).
[OpenCL][Docs] Fixed cross-section reference in OpenCLSupport
Fri, Jan 15, 9:21 AM
Anastasia committed rGd1862a163103: [OpenCL][Docs] Fixed malformed table in OpenCLSupport (authored by Anastasia).
[OpenCL][Docs] Fixed malformed table in OpenCLSupport
Fri, Jan 15, 6:28 AM
Anastasia added a comment to D92277: [OpenCL] Refactor of targets OpenCL option settings.

This patch contains very useful improvements to the design along with the behavioral fixes. I see that it opens more opportunities to bring the implementation in line with the conventional flow and therefore simplify the further development of new functionality. More improvements can be easily built on top of this work. I, therefore, suggest we drive towards finalizing this patch asap and I have made a couple of small comments that should hopefully be quick enough to address.

Fri, Jan 15, 6:02 AM

Thu, Jan 14

Anastasia committed rGadb77a745692: [OpenCL] Improve online documentation. (authored by Anastasia).
[OpenCL] Improve online documentation.
Thu, Jan 14, 6:57 AM
Anastasia closed D93942: [OpenCL] Improve online documentation..
Thu, Jan 14, 6:57 AM · Restricted Project

Fri, Jan 8

Anastasia updated the diff for D93942: [OpenCL] Improve online documentation..

Added the correction from Sven.

Fri, Jan 8, 10:35 AM · Restricted Project
Anastasia committed rG0ef2b68ff063: [OpenCL] Documentation for experimental C++ libs (authored by Anastasia).
[OpenCL] Documentation for experimental C++ libs
Fri, Jan 8, 5:47 AM
Anastasia closed D94188: [OpenCL] Documentation for experimental C++ libraries support.
Fri, Jan 8, 5:46 AM · Restricted Project

Thu, Jan 7

Anastasia added inline comments to D94188: [OpenCL] Documentation for experimental C++ libraries support.
Thu, Jan 7, 10:11 AM · Restricted Project
Anastasia updated the diff for D94188: [OpenCL] Documentation for experimental C++ libraries support.

Addressed review comments

Thu, Jan 7, 10:09 AM · Restricted Project

Wed, Jan 6

Anastasia added a comment to D94188: [OpenCL] Documentation for experimental C++ libraries support.

This patch is uploaded on top of https://reviews.llvm.org/D93942

Wed, Jan 6, 12:55 PM · Restricted Project
Anastasia retitled D94188: [OpenCL] Documentation for experimental C++ libraries support from [OpenCL] add documentation for experimental C++ libraries support to [OpenCL] Documentation for experimental C++ libraries support.
Wed, Jan 6, 12:51 PM · Restricted Project
Anastasia requested review of D94188: [OpenCL] Documentation for experimental C++ libraries support.
Wed, Jan 6, 12:41 PM · Restricted Project
Anastasia committed rG0e874fc014be: [OpenCL] Add clang extension for variadic functions. (authored by Anastasia).
[OpenCL] Add clang extension for variadic functions.
Wed, Jan 6, 12:40 PM
Anastasia closed D94027: [OpenCL] Add clang extension for variadic functions.
Wed, Jan 6, 12:40 PM · Restricted Project
Anastasia committed rG4fde2b6a0c08: [OpenCL] Add clang extension for function pointers. (authored by Anastasia).
[OpenCL] Add clang extension for function pointers.
Wed, Jan 6, 12:40 PM
Anastasia closed D94021: [OpenCL] Add clang extension for function pointers.
Wed, Jan 6, 12:40 PM · Restricted Project
Anastasia added a comment to D94027: [OpenCL] Add clang extension for variadic functions.

LGTM, just one question: I see in the other review you updated clang/test/SemaOpenCL/extension-version.cl. Do you need to do the same here?

Wed, Jan 6, 5:50 AM · Restricted Project
Anastasia updated the diff for D94027: [OpenCL] Add clang extension for variadic functions.

Added missing testing in extension-version.cl

Wed, Jan 6, 5:50 AM · Restricted Project
Anastasia updated the diff for D93942: [OpenCL] Improve online documentation..

Another fix from Marco.

Wed, Jan 6, 4:18 AM · Restricted Project

Tue, Jan 5

Anastasia added a comment to D93942: [OpenCL] Improve online documentation..

(I only discovered the "Suggest Edit" feature halfway through the review; I didn't update the comments I already made as I'm not sure in which format you prefer them?)

Tue, Jan 5, 7:05 AM · Restricted Project
Anastasia added inline comments to D93942: [OpenCL] Improve online documentation..
Tue, Jan 5, 7:04 AM · Restricted Project
Anastasia updated the diff for D93942: [OpenCL] Improve online documentation..

Further change suggested by Marco

Tue, Jan 5, 7:04 AM · Restricted Project
Anastasia updated the diff for D93942: [OpenCL] Improve online documentation..

Addressed comments from Sven.

Tue, Jan 5, 7:01 AM · Restricted Project
Anastasia committed rG6f770292a000: [OpenCL] Restrict pointer to member functions. (authored by Anastasia).
[OpenCL] Restrict pointer to member functions.
Tue, Jan 5, 5:33 AM
Anastasia closed D93958: [OpenCL] Restrict pointer to member functions.
Tue, Jan 5, 5:33 AM · Restricted Project

Mon, Jan 4

Anastasia retitled D94027: [OpenCL] Add clang extension for variadic functions from [OpenCL] Add clang extension for variadic function to [OpenCL] Add clang extension for variadic functions.
Mon, Jan 4, 12:26 PM · Restricted Project
Anastasia requested review of D94027: [OpenCL] Add clang extension for variadic functions.
Mon, Jan 4, 12:26 PM · Restricted Project
Anastasia requested review of D94021: [OpenCL] Add clang extension for function pointers.
Mon, Jan 4, 11:20 AM · Restricted Project

Thu, Dec 31

Anastasia requested review of D93958: [OpenCL] Restrict pointer to member functions.
Thu, Dec 31, 8:00 AM · Restricted Project
Anastasia added inline comments to D93942: [OpenCL] Improve online documentation..
Thu, Dec 31, 5:05 AM · Restricted Project
Anastasia updated the diff for D93942: [OpenCL] Improve online documentation..

Addressed comments from Marco.

Thu, Dec 31, 5:00 AM · Restricted Project
Anastasia accepted D91348: [OpenCL] Warn about side effects for unevaluated vec_step arg.

LGTM, the change seems reasonable. Thanks!

Thu, Dec 31, 3:48 AM · Restricted Project

Wed, Dec 30

Anastasia updated the diff for D93942: [OpenCL] Improve online documentation..

Fixed some build warning/formating issues.

Wed, Dec 30, 6:21 AM · Restricted Project
Anastasia requested review of D93942: [OpenCL] Improve online documentation..
Wed, Dec 30, 4:56 AM · Restricted Project

Fri, Dec 18

Anastasia added inline comments to D76636: [RFC] Added type trait to remove address space qualifier from type.
Fri, Dec 18, 10:26 AM

Dec 11 2020

Anastasia added a comment to D89909: [SYCL] Implement SYCL address space attributes handling.

It was mentioned that the changes in the type system with address spaces is undesirable for SYCL because you indend to parse existing C++ code as-is. This contradicts the intended semantic of address spaces where the whole point of it is to modify the standard types and therefore a compilation of C++ with the standard semantic is only guaranteed when the attribute is not used at all.

Right, but I don't think it's related to the address space attributes. It was mentioned in the context of re-using OpenCL *mode* for SYCL device compilation, which modifies types which does use address space attribute explicitly. "Existing C++ code" doesn't use address space attributes and our solution does differentiate explicitly annotated type. The difference with OpenCL mode is that SYCL doesn't change types by modifying "default" address space attribute and allows conversion from/to "default" address space. As I mentioned in RFC, according to my understanding this patch doesn't contradict Embedded C requirements regarding address space attribute usage. I think the spec allows an implementation to define conversion rules between address spaces and doesn't imply type change based on the declaration scope - it's OpenCL specific behavior.

Ok, if you plan to follow the Embedded C semantic (which your current patch confirms) then you should just reuse the existing target address spaces and extend the implementation with the ability to specify the relation among address spaces. The patch that you have mentioned earlier https://reviews.llvm.org/D62574 is providing this logic already and it looks good aside from testing. I would suggest to discuss with @ebevhan the timeline for committing it as the testing could be done using SYCL implementation. Alternatively, you could consider reusing the relevant parts of the patch if @ebevhan has no objections to that.

Let me check that I understand your proposal correctly.
Do suggest that we use __attribute__((address_space(N))) attribute and define the relation between the set of these? Unfortunately I won't work because SYCL supports multiple targets and we need to use address space maps to correctly map attributes in LLVM IR. Targets might use different different llvm address spaces numbers for memory accessible within a work-item. If I understand it correctly, __attribute__((address_space(N))) will be mapped to addrspace(N) in LLVM and this mapping can't be customized for different targets. Right?

Dec 11 2020, 5:06 AM · Restricted Project, Restricted Project
Anastasia accepted D92721: [PATCH] [clang] Create SPIRABIInfo to enable SPIR_FUNC calling convention.

LGTM! Thanks!

Dec 11 2020, 3:40 AM · Restricted Project

Dec 10 2020

Anastasia updated subscribers of D92277: [OpenCL] Refactor of targets OpenCL option settings.

This looks much cleaner than the current flow! Thanks! We should just figure out a better place for defining the macros (see detailed comment in Targets.cpp).

Dec 10 2020, 10:16 AM
Anastasia committed rGa84599f177a6: [OpenCL] Implement extended subgroups fully in headers. (authored by Anastasia).
[OpenCL] Implement extended subgroups fully in headers.
Dec 10 2020, 8:41 AM
Anastasia closed D92231: [OpenCL] Implement extended subgroups fully in headers.
Dec 10 2020, 8:41 AM · Restricted Project

Dec 8 2020

Anastasia added inline comments to D92231: [OpenCL] Implement extended subgroups fully in headers.
Dec 8 2020, 9:42 AM · Restricted Project
Anastasia updated the diff for D92231: [OpenCL] Implement extended subgroups fully in headers.
  • Set all macros to value 1
Dec 8 2020, 8:13 AM · Restricted Project
Anastasia added inline comments to D92231: [OpenCL] Implement extended subgroups fully in headers.
Dec 8 2020, 7:48 AM · Restricted Project
Anastasia added a comment to D92721: [PATCH] [clang] Create SPIRABIInfo to enable SPIR_FUNC calling convention.

There are many RuntimeCalls that are created in Clang, including _read_pipe, all the OpenMP runtime calls, etc. Shall they all have their calling conventions set from the ABIInfo? Currently those calls are being set to 0.

Dec 8 2020, 7:37 AM · Restricted Project
Anastasia added a comment to D89909: [SYCL] Implement SYCL address space attributes handling.

It was mentioned that the changes in the type system with address spaces is undesirable for SYCL because you indend to parse existing C++ code as-is. This contradicts the intended semantic of address spaces where the whole point of it is to modify the standard types and therefore a compilation of C++ with the standard semantic is only guaranteed when the attribute is not used at all.

Right, but I don't think it's related to the address space attributes. It was mentioned in the context of re-using OpenCL *mode* for SYCL device compilation, which modifies types which does use address space attribute explicitly. "Existing C++ code" doesn't use address space attributes and our solution does differentiate explicitly annotated type. The difference with OpenCL mode is that SYCL doesn't change types by modifying "default" address space attribute and allows conversion from/to "default" address space. As I mentioned in RFC, according to my understanding this patch doesn't contradict Embedded C requirements regarding address space attribute usage. I think the spec allows an implementation to define conversion rules between address spaces and doesn't imply type change based on the declaration scope - it's OpenCL specific behavior.

Dec 8 2020, 7:15 AM · Restricted Project, Restricted Project

Dec 3 2020

Anastasia added a comment to D92231: [OpenCL] Implement extended subgroups fully in headers.

The specification for these extensions was edited by Ben Ashbaugh @Intel. I've asked him about this change.

Sure. Thanks! Happy to hold off committing for a few days to see if Ben has any feedback.

Dec 3 2020, 10:51 AM · Restricted Project

Dec 2 2020

Anastasia added a comment to D92277: [OpenCL] Refactor of targets OpenCL option settings.

Thanks for working on this! It is a very good refactoring that brings OpenCL closer inline with convention design flow

Thanks for feedback. Though the patch indeed requires refactoring and can't be merged as it is, but the main idea still will be the same.

This patch looks inaccurate mainly because I tried to preserve existing interaction with command line. So we should decide the following:

  • Do we need to allow disabling core features via command line?
  • Do we need to allow target disabling core features (NVPTX does so for OpenCL C 2.0 now)?

I personally think that we shouldn't allow doing so in both cases since it violates the specification of OpenCL C 2.0. In this way the change will be much cleaner because we can use only TargetInfo::getOpenCLFeatureDefines( LangOptions) to define macros for core features and set them in OpenCLOptions::addSupport(StringMap<bool> TargetOpts, LangOptions).

Dec 2 2020, 8:43 AM

Dec 1 2020

Anastasia accepted D92406: [OpenCL] Add some more kernel argument tests.

LGTM! Thanks

Dec 1 2020, 10:39 AM · Restricted Project
Anastasia added a comment to D92231: [OpenCL] Implement extended subgroups fully in headers.

The specification for these extensions was edited by Ben Ashbaugh @Intel. I've asked him about this change.

Dec 1 2020, 4:19 AM · Restricted Project

Nov 30 2020

Anastasia added a comment to D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h.

We probably don't want to hold up this patch until we have more thorough testing in place, since we don't even have any concrete plans for such testing at the moment.

Well accepting such a significant change that implements the entire functionality of a new standard without a single test doesn't sound a workable approach. We should at least add some amount of testing in existing lib/Headers/opencl-c.h. However, instead of fighting with ad-hoc testing, it might save us time if we agree on a strategy and just do the testing properly. After all it is relatively clear what is to be tested. The issue is mainly with the testing time and amount of overloads to be tested. It should not be too difficult to guard such testing by a cmake option to avoid long execution time of default testing target of clang. However, it might be time-consuming to list all overloads. If we could use C++ templates it would help.

Perhaps my assumption that we do not want to hold up this patch until we have more thorough testing in place was wrong then. I agree it would be good to have better header testing of the header. I merely wanted to point out that testing the header thoroughly is a substantial piece of work. So if we want to have such testing in place first, we need to delay this patch and start a thread outside of this review.

Nov 30 2020, 10:57 AM · Restricted Project
Anastasia added a comment to D92277: [OpenCL] Refactor of targets OpenCL option settings.

FYI I just noticed this review is public, but not linked to cfe-commits. Not sure it is intentional though.

Nov 30 2020, 10:25 AM
Anastasia added a comment to D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h.

I am still unclear what should we do with testing though.

We probably don't want to hold up this patch until we have more thorough testing in place, since we don't even have any concrete plans for such testing at the moment.

Nov 30 2020, 9:41 AM · Restricted Project
Anastasia added a comment to D92277: [OpenCL] Refactor of targets OpenCL option settings.

Thanks for working on this! It is a very good refactoring that brings OpenCL closer inline with convention design flow. I also suggest removing the dependence on LangOpts. See detailed comments.

Nov 30 2020, 8:48 AM
Anastasia accepted D92091: [OpenCL] Allow pointer-to-pointer kernel args beyond CL 1.2.

The testing can be improved before committing!

Nov 30 2020, 3:19 AM · Restricted Project

Nov 27 2020

Anastasia requested review of D92244: [OpenCL] Prevent adding vendor extensions for all targets.
Nov 27 2020, 12:30 PM
Anastasia requested review of D92231: [OpenCL] Implement extended subgroups fully in headers.
Nov 27 2020, 7:20 AM · Restricted Project
Anastasia added a comment to D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h.

Thanks! This looks much cleaner.

Nov 27 2020, 6:03 AM · Restricted Project
Anastasia added a comment to D89909: [SYCL] Implement SYCL address space attributes handling.

Did anyone conclude there that the language address spaces should be added for SYCL? I can't see any of this. In fact I don't even think there was any conclusion on the RFC. You should first make your design clear and agreed before going ahead with the implementation. I personally still think that address space AST attribute is not the right path for SYCL. And I haven't seen any evidence that it is the best solution to what you are trying to achieve.

I'll try to summarize my understanding of the current state. So far there were three options discussed in the mailing list thread:

  1. We started with re-using OpenCL address space attributes (https://reviews.llvm.org/D80932), but there was a concern regarding deviation from OpenCL semantics in SYCL mode for these attributes. In same review, there was a proposal to implement a separate set of attributes to address this concern, which lead to the solution #2.
  2. This patch (https://reviews.llvm.org/D89909) implements proposal from https://reviews.llvm.org/D80932 review comments to add SYCL specific attributes to avoid interference with OpenCL mode.
  3. Bevin Hansson suggested to resolve semantic difference between SYCL and OpenCL mode for OpenCL address space attributes handling by re-using this work: https://reviews.llvm.org/D62574, which provides an infrastructure to customize address space conversion rules based on information available in ASTContext. This seems to be viable approach to implement the difference between OpenCL and SYCL modes.

Open source SYCL implementation from https://github.com/intel/llvm/tree/sycl currently implements option #1 and option #2 was used in the past, so I'm quite confident that these will work. I haven't tried to prototype option #3, but as mentioned in RFC, https://reviews.llvm.org/D62574 provides necessary infrastructure to customize address space attributes already supported by the compiler.
As we already agreed in the RFC, we should have address space attributes in AST for SYCL and the only question we need to answer is whether SYCL mode can re-using existing attributes or add something new.

My current plan is to continue with option #2, unless Bevin's patch is going to be committed soon. I'd like to have something committed to unblock other changes relying on address space attributes. Let me know if you have any concerns regarding this plan.

Nov 27 2020, 4:30 AM · Restricted Project, Restricted Project
Anastasia added inline comments to D89869: [OpenCL] Define OpenCL feature macros for all versions.
Nov 27 2020, 3:30 AM

Nov 26 2020

Anastasia added a comment to D89869: [OpenCL] Define OpenCL feature macros for all versions.

After a recent discussion with Khronos on https://github.com/KhronosGroup/OpenCL-Docs/issues/500 and more thinking I start to realize that there is absolutely nothing new in OpenCL 3.0 features as it isn't a new concept at all. They use to be explained as optional core features in the extension spec from early spec versions. So I think what have changed mainly in OpenCL 3.0 is that the features that were core have now become optional while the current design in clang assumes that core features can't become optional. I feel this is what we should reflect better in the design right now. So the categories we have are:

  • extension (conditionally supported, can have pragma to alter certain functionality)
  • optional feature (conditionally supported, pragma has no effect)
  • core feature (unconditionally supported, pragma has no effect)
Nov 26 2020, 11:31 AM
Anastasia added a comment to D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros.

When reading the documentation [1] for -cl-ext (which I've never used so far), I've noticed nothing is said about non-standard configurations (such as disabling cl_khr_depth_images with CL2.0). Quickly testing this shows that options can be specified to produce non-standard behaviour, as shown by https://godbolt.org/z/1Yz1Md.

Is it intentional that -cl-ext allows such non-standard behaviour? (This question is not necessarily address to @Anastasia.)
/If so/, then these statements

Defining __undef_cl_khr_depth_images can alter the default behavior of the predefined macro. This is equivalent to passing -cl-ext=-cl_khr_depth_images.

and

cl_khr_depth_images is a core functionality of CL2.0 and thefore defining __undef_cl_khr_depth_images doesn't modify the default behavior.

are slightly contradicting each other: the approach with __undef macros seems to ensure a more conformant behaviour.

I'm mainly asking for clarification in order to know in which direction we want to go, as one could also argue the present documentation doesn't imply non-standard behaviour is desired and that the current implementation of -cl-ext is buggy.

[1] https://clang.llvm.org/docs/UsersManual.html#cmdoption-cl-ext

Nov 26 2020, 9:25 AM
Anastasia accepted D90928: [OpenCL] Check for extension string extension lookup.

Great! LGTM! Thanks!

Nov 26 2020, 9:13 AM · Restricted Project
Anastasia added inline comments to D92091: [OpenCL] Allow pointer-to-pointer kernel args beyond CL 1.2.
Nov 26 2020, 8:43 AM · Restricted Project

Nov 25 2020

Anastasia added a comment to D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h.

Btw how about making some checks simpler. We could always define feature macros __opencl_c_atomic_scope_device, __opencl_c_generic_address_space for OpenCL 2.0 or C++ for OpenCL . Then everywhere we would only need to check feature macros instead of language versions and feature macros together.

#if !(defined()) && (defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0))
#define __opencl_c_atomic_scope_device      1
#define __opencl_c_generic_address_space   1
...
#endif

I like this idea but my only worry was about leaking those definitions to the user source when the system hasn't defined them.

i.e. a CL2.0 user now sees __opencl_c_generic_address_space, writes code to use that macro, but it fails on other OpenCL C implementations.

But if we are going to have feature code that works like that then I'm fine with going ahead and changing things to look like this.

Nov 25 2020, 2:31 AM · Restricted Project

Nov 24 2020

Anastasia accepted D92033: [OpenCL] Move kernel arg type tests into one file.

LGTM! Thanks

Nov 24 2020, 7:53 AM · Restricted Project
Anastasia added a comment to D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros.

So if I understand it well you are suggesting to perform a check for every parsed macro definition that would identify whether the macro has a name matching what is provided in -cl-ext=? Then if the name matches it would check whether the macro should be defined or not?

No, sorry for confusing. There is no need to check every parsed macro. For example an internal pragma can be used for these purposes (#pragma OPENCL EXTENSION all : undef in example below). So imagine if we can differentiate header-only extensions and features and all of them are defined in header. After the list of definitions the special pragma is used and is a processed in a way that it inserts undef for each macro definition which relates to header-only extension/feature and was turned off with the option (-cl-ext=-cl_khr_depth_images):

#define cl_khr_depth_images 1
#define cl_khr_fp64 1
#define cl_khr_mipmap_image  1
...
#pragma OPENCL EXTENSION all : undef

However, this might be hard to maintain and I'm not sure yet that this is even legally to do in current Preprocessor design, but this is at least more scalable than adding #if defined(__undef_ for each extension in the end of the header. Nevertheless, that's what I meant about preserving the current interface.

Nov 24 2020, 7:27 AM
Anastasia updated subscribers of D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h.

It will need some decent review and maybe some testing by other CL users to make sure I've haven't messed up when used with a fully featured CL 3.0 stack.

Nov 24 2020, 5:57 AM · Restricted Project
Anastasia added a comment to D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h.

this file has never been clang-format clean, happy to make changes if reviewer decides they are needed.

Nov 24 2020, 5:33 AM · Restricted Project
Anastasia updated subscribers of D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h.

I assume your change depends on some other changes that add the feature macro definitions. So I think we should link the other changes in at some point. CCing to @azabaznov here who is working on the features too.

Nov 24 2020, 5:29 AM · Restricted Project
Anastasia added a comment to D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h.

Btw how about making some checks simpler. We could always define feature macros __opencl_c_atomic_scope_device, __opencl_c_generic_address_space for OpenCL 2.0 or C++ for OpenCL . Then everywhere we would only need to check feature macros instead of language versions and feature macros together.

Nov 24 2020, 5:27 AM · Restricted Project

Nov 23 2020

Anastasia added a reviewer for D90928: [OpenCL] Check for extension string extension lookup: Anastasia.
Nov 23 2020, 11:03 AM · Restricted Project
Anastasia added inline comments to D90928: [OpenCL] Check for extension string extension lookup.
Nov 23 2020, 11:03 AM · Restricted Project
Anastasia added a comment to D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros.

Perhaps if you give me an example it would help to understand

I was meant that this can potentially be used to undefine macros inside clang directly. In this case there will no need to add a lot of conditional preprocessor directives in the header, also the existing interface (-cl-ext=-cl_khr_depth_images) will be preserved. So for example in the header there was specified an extension definition. Can undef directive be allocated and bound to a specific source location right after extension definition if -cl-ext=-cl_khr_depth_images was specifed:

#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0) || \
    (__OPENCL_C_VERSION__ >= CL_VERSION_1_2  && defined(__SPIR__) )
#define cl_khr_depth_images 1
#endif

// Bind undef directive here

I understand that this sounds tricky, but preserving interface sound reasonable for me.

Nov 23 2020, 10:47 AM
Anastasia added a comment to D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros.

Yes, in general this approach looks good to me conceptually. I have two suggestions:

  1. As we discussed, the term core functionality should be revisited here. There's no clear meaning about that in the spec and I think interpreting it as supported by default is a little dangerous. So core (AFAIK) means that it was just promoted to a core specification thus is still remains optional by targets.
Nov 23 2020, 9:10 AM
Anastasia added a comment to D90928: [OpenCL] Check for extension string extension lookup.

Do you think we could improve testing? I presume there is something that triggers a failure without your change...

I'm not really sure how to test this code. Best I can tell, there's no way for the clang executable to call these functions with invalid strings. I only ran into the seg faults when I was programmatically setting options using the clang API.

Nov 23 2020, 8:18 AM · Restricted Project

Nov 19 2020

Anastasia added a comment to D90928: [OpenCL] Check for extension string extension lookup.

LGTM for the fix! Do you think we could improve testing? I presume there is something that triggers a failure without your change...

Nov 19 2020, 8:34 AM · Restricted Project
Anastasia added inline comments to D89869: [OpenCL] Define OpenCL feature macros for all versions.
Nov 19 2020, 5:17 AM

Nov 17 2020

Anastasia added inline comments to D89869: [OpenCL] Define OpenCL feature macros for all versions.
Nov 17 2020, 5:34 AM
Anastasia updated the diff for D91538: [RFC][OpenCL] Make Tablegen header work with extensions that are not added in clang.

Added full diffs.

Nov 17 2020, 3:41 AM
Anastasia updated the diff for D91534: [RFC][OpenCL] Add new diagnostic for extension pragma with no effect.

Added full diff.

Nov 17 2020, 3:40 AM
Anastasia updated the diff for D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros.
  • Added full diffs.
Nov 17 2020, 3:33 AM

Nov 16 2020

Anastasia added a comment to D89869: [OpenCL] Define OpenCL feature macros for all versions.

Addressed all concerns except replacing __opencl_c_int64 definition into header. The reason for this as follows: this macro should be predefined regardless if clang includes default header or not.

FYI clang doesn't provide full support of OpenCL without the header. In fact, there is ongoing work on making the header included by default without any flag. But I can see that this functionality is related to the frontend and not something that is simply added via libraries so I don't have strong objections for adding it in the clang source code. However, the macro can be added without registering the new extension (see how __IMAGE_SUPPORT__ is added for example). Right now you are adding a pragma and a target setting that targets can modify without any effect, so I would like to avoid these.

My preference would be if you prepare a separate patch for this. Smaller selfcontained patches are easier and faster to review and also it reduces gaps in testing.

Nov 16 2020, 6:30 AM
Anastasia added a reviewer for D91534: [RFC][OpenCL] Add new diagnostic for extension pragma with no effect: jvesely.
Nov 16 2020, 6:24 AM
Anastasia updated subscribers of D91538: [RFC][OpenCL] Make Tablegen header work with extensions that are not added in clang.
Nov 16 2020, 6:24 AM
Anastasia added reviewers for D91538: [RFC][OpenCL] Make Tablegen header work with extensions that are not added in clang: svenvh, mantognini, azabaznov.
Nov 16 2020, 6:24 AM
Anastasia added reviewers for D91534: [RFC][OpenCL] Add new diagnostic for extension pragma with no effect: mantognini, svenvh, azabaznov, yaxunl.
Nov 16 2020, 6:23 AM
Anastasia updated subscribers of D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros.
Nov 16 2020, 6:22 AM
Anastasia retitled D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros from [RFC][OpenCL] Provide mechanisms for defining extensions macros to [RFC][OpenCL] Provide mechanisms for defining extension macros.
Nov 16 2020, 6:22 AM
Anastasia updated the summary of D91538: [RFC][OpenCL] Make Tablegen header work with extensions that are not added in clang.
Nov 16 2020, 6:02 AM
Anastasia retitled D91538: [RFC][OpenCL] Make Tablegen header work with extensions that are not added in clang from [RFC][OpenCL] Make tablegen header work with extension that are not added in clang to [RFC][OpenCL] Make Tablegen header work with extensions that are not added in clang.
Nov 16 2020, 6:00 AM
Anastasia requested review of D91538: [RFC][OpenCL] Make Tablegen header work with extensions that are not added in clang.
Nov 16 2020, 5:53 AM
Anastasia updated the summary of D91534: [RFC][OpenCL] Add new diagnostic for extension pragma with no effect.
Nov 16 2020, 5:26 AM
Anastasia retitled D91534: [RFC][OpenCL] Add new diagnostic for extension pragma with no effect from [RFC][OpenCL] Add new diagnostic for OpenCL pragma with no effect to [RFC][OpenCL] Add new diagnostic for extension pragma with no effect.
Nov 16 2020, 5:23 AM
Anastasia requested review of D91534: [RFC][OpenCL] Add new diagnostic for extension pragma with no effect.
Nov 16 2020, 5:22 AM
Anastasia retitled D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros from [RFC][OpenCL] Provide mechnisms for defining extensions macros to [RFC][OpenCL] Provide mechanisms for defining extensions macros.
Nov 16 2020, 5:05 AM
Anastasia updated the summary of D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros.
Nov 16 2020, 4:48 AM
Anastasia updated the summary of D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros.
Nov 16 2020, 4:44 AM
Anastasia updated the summary of D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros.
Nov 16 2020, 4:43 AM
Anastasia updated the summary of D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros.
Nov 16 2020, 4:42 AM