Page MenuHomePhabricator

Builtins: Add some v2f16 variants
Needs ReviewPublic

Authored by arsenm on Aug 5 2019, 8:42 AM.

Details

Summary

This will avoid hacks in the OpenCL builtin library.

Diff Detail

Event Timeline

arsenm created this revision.Aug 5 2019, 8:42 AM

Hmm. I think there are two reasonable concerns here, both arising from the fact that these names occupy (the builtin namespace parallel to) a namespace that the committee might want to occupy in the future:

  • Is a v2fN suffix sufficiently likely to avoid interfering with this namespace? If there's an *exact* collision, that might be okay if the builtin actually matches the standard behavior; my concern is that they might conflict. (Different orderings of element count and element type? Unrelated functions ending in v?)
  • Is it a good idea to introduce builtins in this namespace that traffic in our current vector types instead of a potential future standard-blessed vector type?

You might want to look into ISO/IEC TS 19570 - 2018 to see what they're thinking. If it looks like there's a potential conflict, then maybe these should have an OpenCL-specific prefix on them.

Hmm, sorry, that's a C++ spec, and it looks like the (abandoned) C attempt at data parallelism (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2081.htm) doesn't introduce vector types.

Talking about it internally, I think there are two reasonable approaches:

  • Add an OpenCL prefix to the builtin, e.g. __builtin_cl_floorv216. This isn't great because it feels like it's heading towards a world with a million builtins for different element types and vector sizes.
  • Make the builtin type-generic, e.g. __builtin_generic_floor (or __builtin_tgmath_floor), where the builtin allows an arbitrary floating-point argument (or vector thereof).
scanon added a subscriber: scanon.Aug 6 2019, 1:02 PM

Strongly agree with what @rjmccall said. If we can make these generic builtins instead of ending up with O(100) variants of each math operation, that would make life immensely nicer.

scanon added a comment.EditedAug 6 2019, 1:03 PM

(Ideally we would just call them e.g. __builtin_floor, but that would be source-breaking. __builtin_tgmath_floor seems like a good compromise.)