Skip to content

[SYCL] Allow recursive function calls in a constexpr context. #2105

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.

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 2 commits into from
Jul 15, 2020

Conversation

erichkeane
Copy link
Contributor

It doesn't really make sense to restrict recursion for something that is
forced to be evaluated at constexpr time, since this is a restriction
due to device limitations. This patch creates a constexpr-context count
for all of the constexpr contexts I could create, and creates a counter.

It is implemented this way to permit a future implementer to add other
diagnostics (such as DLLImport?) as permissible in constexpr.

This was requested in #2104

It doesn't really make sense to restrict recursion for something that is
forced to be evaluated at constexpr time, since this is a restriction
due to device limitations.  This patch creates a constexpr-context count
for all of the constexpr contexts I could create, and creates a counter.

It is implemented this way to permit a future implementer to add other
diagnostics (such as DLLImport?) as permissible in constexpr.
@erichkeane
Copy link
Contributor Author

Ping @premanandrao I think the other two are on vacation today :)

Also, I've got no idea on the Jenkins failure, it claims that test_hierarchical_opencl failed, yet there is nothing in this patch that could have caused that.

@elizabethandrews
Copy link
Contributor

elizabethandrews commented Jul 15, 2020

Ping @premanandrao I think the other two are on vacation today :)

I'm working till noon to attend your training today!

Also, I've got no idea on the Jenkins failure, it claims that test_hierarchical_opencl failed, yet there is nothing in this patch that could have caused that.

I didn't see this fail. The links led me to this failing test - test_vector_swizzles_opencl. I don't think this is related to your patch either.

@elizabethandrews
Copy link
Contributor

LGTM but @premanandrao can you also take a look?

Copy link
Contributor

@premanandrao premanandrao left a comment

Choose a reason for hiding this comment

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

LGTM.

@premanandrao
Copy link
Contributor

Ping @premanandrao I think the other two are on vacation today :)

I thought I had approved it yesterday. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants