WIP: xe: use double with post-ops with f64 to preserve accuracy#4746
WIP: xe: use double with post-ops with f64 to preserve accuracy#4746
Conversation
|
make test |
Simonsays095
left a comment
There was a problem hiding this comment.
There are still many other OpenCL kernels that use float instead of POST_OP_DATA_T. I guess those will be handled separately?
| POST_OP_DATA_T dst_data; | ||
| #if WITH_SUM | ||
| dst_data = convert_float(DATA_TO_REF(C[dst_off])); | ||
| dst_data = (POST_OP_DATA_T)DATA_TO_REF(C[dst_off]); |
There was a problem hiding this comment.
DATA_TO_REF here will convert to float. We either need to change DATA_TO_REF for f64 to be convert_double, or carve out an exception via macros here.
There was a problem hiding this comment.
Likely DATA_TO_REF should be replaced with FLT_ACC_DATA_T which is already used in this file. Alternatively, we can just refactor this to use the load/store API so that we don't need to go over this with a fine-grained comb looking for invalid casts.
As an aside, currently we have 3 different floating point accumulator types DATA_TO_REF, POST_OP_DATA_T, and FLT_ACC_DATA_T. I am not convinced we really need all of these.
There was a problem hiding this comment.
Thanks for the catch, let me move the PR to WIP for now. I want to try to migrate REF family of macros to load/store. Eliminating extra macros would be nice.
There was a problem hiding this comment.
Just in case this is helpful, here as a sample commit porting gemm to use the load store interface.
MFDNN-14668