libclc: Fix rounding during type conversion

[daniels]

Summary

The rounding during type conversion uses multiple conversions, selecting
between them to try to discover if rounding occurred. This appears to
not have been tested, since it would generate code of the form:

float convert_float_rtp(char x)
{
  float r = convert_float(x);
  char y = convert_char(y);
  [...]
}

which will access uninitialised data. The idea appears to have been to
have done a char -> float -> char roundtrip in order to discover the
rounding, so do this.

Discovered by inspection.

Signed-off-by: Daniel Stone <[e-mail]>

[daniels]

I don't have a good way to test this, but tbh I don't think anyone else does either, given it came from POCL so has been broken for as long as libclc's had any support for it. Looking into POCL just now, it looks like they applied the same fix a few years ago. There are other fixes as well which might be good to take up, but I'm not going to propose any invasive changes, since SPIR-V doesn't (can't) use these helpers at all: the LLVM-SPIRV-Translator specifically looks for calls to the convert_* functions and replaces them with equivalently-decorated SPIR-V OpConvert*.

[jvesely]

I think these routines need to be rewritten and tested with CTS on platforms that use them.

[daniels]

I totally agree, but I don't have those platforms so I won't be progressing this myself.

[alan-baker]

This definitely looks like the correct fix. I had originally included it in D94013, but it makes more sense as separate commit.

[jvesely]

while the fix is clearly correct, it'd be nice to do a test run and report changes.
I can run it on my carrizo machine, but the conversion tests take more than a day.

[jvesely]

it took a while to test both versions. this patch fixes the following conversion tests:

	 *** 622) convert_floatn_rtp( uintn ) FAILED ** 
	 *** 627) convert_floatn_rtp( intn ) FAILED ** 
	 *** 639) convert_floatn_rtz( doublen ) FAILED ** 
	 *** 649) convert_floatn_rtz( longn ) FAILED ** 
	 *** 689) convert_doublen_rtz( doublen ) FAILED **

The number of failed cases goes down from 42 to 37

[daniels]

In D81999#2599293, @jvesely wrote:

it took a while to test both versions. this patch fixes the following conversion tests:

	 *** 622) convert_floatn_rtp( uintn ) FAILED ** 
	 *** 627) convert_floatn_rtp( intn ) FAILED ** 
	 *** 639) convert_floatn_rtz( doublen ) FAILED ** 
	 *** 649) convert_floatn_rtz( longn ) FAILED ** 
	 *** 689) convert_doublen_rtz( doublen ) FAILED **

The number of failed cases goes down from 42 to 37

Great, thanks ... is there any reason not to land this now?

[tstellar]

@daniels Do you have commit access?

[daniels]

In D81999#2952113, @tstellar wrote:

@daniels Do you have commit access?

Nope, I need someone to land this for me please.

libclc/generic/lib/gen_convert.py

elif mode == '_rtp':
       print("  x = ceil(x);");
     elif mode == '_rtn':
       print("  x = floor(x);");
     print("  return convert_{DST}{N}{S}(x);".format(DST=dst, N=size, S=sat))
   elif mode == '_rte':
     print("  return convert_{DST}{N}(x);".format(DST=dst, N=size))
   else:
     print("  {DST}{N} r = convert_{DST}{N}(x);".format(DST=dst, N=size))
-    print("  {SRC}{N} y = convert_{SRC}{N}(y);".format(SRC=src, N=size))
+    print("  {SRC}{N} y = convert_{SRC}{N}(r);".format(SRC=src, N=size))
     if mode == '_rtz':
       if src in int_types:
         print("  {USRC}{N} abs_x = abs(x);".format(USRC=unsigned_type[src], N=size))
         print("  {USRC}{N} abs_y = abs(y);".format(USRC=unsigned_type[src], N=size))
       else:
         print("  {SRC}{N} abs_x = fabs(x);".format(SRC=src, N=size))
         print("  {SRC}{N} abs_y = fabs(y);".format(SRC=src, N=size))
       print("  return select(r, nextafter(r, sign(r) * ({DST}{N})-INFINITY), convert_{BOOL}{N}(abs_y > abs_x));"