[HEADER] Overloadable function candidates for half/double types
Needs ReviewPublic

Authored by sidorovd on Aug 28 2018, 1:55 AM.

Details

Reviewers
yaxunl
Anastasia
Summary

Overloadable function candidate should not be viable if it uses extensions (Half or Double type) that are not enabled or supported.

Patch by Egor Churaev

Diff Detail

sidorovd created this revision.Aug 28 2018, 1:55 AM
sidorovd updated this revision to Diff 162812.Aug 28 2018, 2:03 AM
krisb added a subscriber: krisb.Aug 28 2018, 2:27 AM
krisb removed a subscriber: krisb.
krisb added a subscriber: krisb.
Anastasia added inline comments.Aug 29 2018, 6:47 AM
lib/Sema/SemaOverload.cpp
6063

I would imagine if extension isn't supported the candidate function with data type defined by extension shouldn't be available at all during compilation?

Also is there any good way to generalize this for all types and extensions including vendor ones that are added with the pragmas?
https://clang.llvm.org/docs/UsersManual.html#opencl-extensions

test/SemaOpenCL/half-double-overload.cl
8

I think it will be clearer if candidates with non-half parameters moved out of extension enabled block.

19

Wondering if better message could be:

candidate unavailable as it requires OpenCL extension to be disabled

Could it also print the extension name?

sidorovd marked an inline comment as done.Sep 6 2018, 1:52 AM
sidorovd added inline comments.
lib/Sema/SemaOverload.cpp
6063

I would imagine if extension isn't supported the candidate function with data type defined by extension shouldn't be available at all during compilation?

Yes, you are right.

Also is there any good way to generalize this for all types and extensions including vendor ones that are added with the pragmas?

https://clang.llvm.org/docs/UsersManual.html#opencl-extensions

There might be a way but I can't properly answer this question right now. By default types and extensions aren't associated to each other.

test/SemaOpenCL/half-double-overload.cl
19

Sounds good. And I'd prefer to split it into another patch, because it would affect unrelated to the change tests and also require changes in Sema to allow extension name printing.

sidorovd updated this revision to Diff 164169.Sep 6 2018, 1:54 AM
sidorovd edited the summary of this revision. (Show Details)
Anastasia added inline comments.Sep 10 2018, 10:36 AM
lib/Sema/SemaOverload.cpp
6063

We have a map of types and associated extensions with them in OpenCLTypeExtMap in Sema.h... not sure what would be the cost of such generic check though.

test/SemaOpenCL/half-double-overload.cl
19

Ok, makes sense for this to be a separate change!

For now this patch is on hold since it adds no new functionality. Its future will be decided after @asavonic finished https://reviews.llvm.org/D51544