Skip to content

Fix rocblas_bfloat16 conversions and add a __device__ __host__ rocblas_abs() function#678

Merged
leekillough merged 3 commits into
ROCm:developfrom
leekillough:bfloat16_to_float
Aug 31, 2019
Merged

Fix rocblas_bfloat16 conversions and add a __device__ __host__ rocblas_abs() function#678
leekillough merged 3 commits into
ROCm:developfrom
leekillough:bfloat16_to_float

Conversation

@leekillough
Copy link
Copy Markdown
Contributor

I will have to do build and test run manually.

I'm not 100% sure this fixes it, but it should.

@leekillough leekillough changed the title Replace explicit conversion of bloat16 to float and double with impicit conversion of bloat16 to float Replace explicit conversion of bloat16 to float and double with implicit conversion of bloat16 to float Aug 30, 2019
@leekillough leekillough changed the title Replace explicit conversion of bloat16 to float and double with implicit conversion of bloat16 to float Fix rocblas_bfloat16 conversions and add a __device__ __host__ rocblas_abs() function Aug 30, 2019
@leekillough
Copy link
Copy Markdown
Contributor Author

There were three problems addressed:

  1. The rocblas_bfloat16 type should be allowed to be implicitly converted to float, not just explicitly converted to float or double. There is no loss in the conversion from rocblas_bfloat16 to float, so it can be allowed implicitly. If converted to double, it will be implicitly converted to float first.
  2. The std::abs overload for rocblas_bfloat16 was buggy, using the integer value of data & 0x7fff converted to rocblas_bfloat16 as the return value, instead of the absolute value of the BF16 value.
  3. The std::abs function was being called from __device__ functions, which isn't generally allowed. As a result, when trying to call std::abs from __device__ functions with float or double arguments, the compiler only saw the rocblas_bfloat16 overload of std::abs as a viable __device__ overload candidate function. It then tried to convert the float or double argument to rocblas_bfloat16, but it couldn't, since rocblas_bfloat16's constructor is explicit (a conversion to rocblas_bfloat16 from float or double is lossy, and thus should not be allowed implicitly).

The fix:

  1. Allow rocblas_bfloat16 to be implicitly converted to float, without loss. The conversion to double will then be automatic by the compiler, first converting it to float and then to double.
  2. Remove the std::abs overloads for rocblas_bfloat16.
  3. Add a rocblas_abs function to be callable from __device__ or __host__ functions, and put the rocblas_bfloat16 overfload there.
  4. Change ROT*g functions to use rocblas_abs instead of std::abs.

Copy link
Copy Markdown
Contributor

@AlexBrownAMD AlexBrownAMD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@leekillough leekillough merged commit 253fbf6 into ROCm:develop Aug 31, 2019
@leekillough leekillough mentioned this pull request Sep 1, 2019
@leekillough leekillough deleted the bfloat16_to_float branch December 27, 2019 00:29
mlse-lib-jenkins pushed a commit that referenced this pull request Jun 4, 2021
* extend range of single block and inc1
* add testing for better case coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants