cl_khr_fp16 is poorly tested

[kpet]

Missing tests in most of the suites:

• [x] bruteforce
• [x] conversions
• [ ] relationals
• ... and probably others

[EwanC]

Trying to figure out exactly what folders would be the most work to properly test this extension. Any insights welcome.

Bruteforce

Rather than reimplementing all the reference functions with a 32-bit single precision overload to verify half against, we could use Ulp_Error_Half with the existing double references. Would that be admissible?

For functions which take a single parameter we can resonably test every possible 16-bit input. How many inputs should we test for functions which take multiple parameters? Not sure how this is currently done.

Geometric

Looks a pain, seperate ~1000 line files for single and double precision, with a lot of duplication. C++ files rather than C, so scope for some refactoring but could be a fair bit of work.

Relationals

Same as geometrics but not as large, seperate files for float and double which could be cleaned up into a single file for all floating point types.

Verification functions on host won't be able to use half, convert to float with half2float and do comparison there instead.

Conversions

Already a mass of function definitions enumerating all the possibilites that will need to be added to. Definetelty scope for refactoring this code to use templates.

This test already takes a long time, could be an issue if increase in combinations makes this worse

Commonfns

Validation functions use type of same precision on, but host code won't be able to use half, upcast to float with half2float and do comparison there instead.

Basic

• async_work_group_strided_copy
• async_work_group_copy
• vload/store
• astype

Select

Glance through suggests not much to add and we can us cl_ushort & cl_short in reference functions.

Printf

Doesn't look like too much work other than adding the format specifiers

SPIR

Regenerate zips and update khr.csv?

SPIRV New

Don't know much about these.

[kpet]

Thanks for the initial analysis, that's great input.

I think we've reached the point where we need to take a step back and figure out how we want the code of these individual tests to be structured overall. We can't keep copy/pasting and adding special cases :). If there are specific refactorings you have in mind that you think would make it easier to add FP16 support, feel free to suggest them here, create individual issues or send PRs directly.

[EwanC]

Addressing the copy/paste situation definitely sounds like a good approach long term. The worst offender seems to be conversions, in particular basic_test_conversions.c, so I've come up with an idea about that.

I think we need to change this from a C file to C++ so we can use templates to reduce the duplication, since this code doesn't use any of our C entry points I don't think that should be a problem.

When I wrote some unit testing of conversions internally for half, I used a templated struct resembling the below, with Strong Types to differentiate between cl_half and cl_ushort in template instantiations.

template <typename StrongFrom, typename StrongTo>
struct ConvertHelper {
  using WeakTo = typename StrongTo::WrappedT;
  using WeakFrom = typename StrongFrom::WrappedT;

  // Reference function
  static WeakTo reference(const WeakFrom x, const RoundingMode rounding, const bool saturated);

  // Check for undef behaviour if validation with reference failed
  static bool undef(const weakFrom, const bool saturated);

  // Used to check for FTZ 
  static bool denormal(const weakFrom x);
};

Then defined various partial specializations for different cases like int->float, smaller float->larger float, etc. Which is where we'd add the fp16 conversion references.

Using saturated conversions as an example from the CTS if we use a similar technique, all the foo2bar_sat_many definitions could be replaced with a function like

template<typename StrongFrom, typename StrongTo>
convert_test(void * out, void* in, size_t n) {
  StrongFrom::WrappedT* in_ptr = (StrongFrom::WrappedT*) in;
  StrongTo::WrappedT* out_ptr = (StrongTo::WrappedT*) out;

  for (size_t i = 0; i < n; i++) {
    ConvertHelper<StrongFrom, StrongTo>::reference(out_ptr + i, in_ptr + i, RoundingMode::default, true); }
  }
}

Which we'd instate in gSaturatedConversions

Convert gSaturatedConversions[kTypeCount][kTypeCount] = {
  convert_test<UChar, UChar>, ..., convert_test<Long, Long>
};

Hacking together a prototype would be the best way to see if this is really suitable, but finding the time for that is a different matter. So unfortunately can't commit to opening a PR with this in the short term.

[gwawiork]

Regrading Bruteforce I pushed our proposal #529 for some functions and fp16. We have more changes in other files: binary_two_results binaryOperator ternary unary_two_results unary_two_results_i

However we need some more time to debug and fix errors we see.

[bashbaug]

Note, #1433 should help to close some of the fp16 gaps if we can get it polished up and merged.

[neiltrevett]

Request suggested test plan from Mobica before work start