Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.3.X] TRMM functions do not have correct correspondence in hipBLAS #524

Closed
emankov opened this issue Sep 19, 2022 · 4 comments
Closed
Assignees

Comments

@emankov
Copy link

emankov commented Sep 19, 2022

hipBLAS TRMM functions hipblasStrmm, hipblasDtrmm, hipblasCtrmm, hipblasZtrmm do not match neither cublas TRMM functions, nor cublas TRMM _v2 functions.

For instance:

cublasStrmm:

void CUBLASWINAPI cublasStrmm(char side,
                              char uplo,
                              char transa,
                              char diag,
                              int m,
                              int n,
                              float alpha,
                              const float* A,
                              int lda,
                              float* B,
                              int ldb);

cublasStrmm_v2:

CUBLASAPI cublasStatus_t CUBLASWINAPI cublasStrmm_v2(cublasHandle_t handle,
                                                     cublasSideMode_t side,
                                                     cublasFillMode_t uplo,
                                                     cublasOperation_t trans,
                                                     cublasDiagType_t diag,
                                                     int m,
                                                     int n,
                                                     const float* alpha, /* host or device pointer */
                                                     const float* A,
                                                     int lda,
                                                     const float* B,
                                                     int ldb,
                                                     float* C,
                                                     int ldc);

hipblasStrmm:

HIPBLAS_EXPORT hipblasStatus_t hipblasStrmm(hipblasHandle_t    handle,
                                            hipblasSideMode_t  side,
                                            hipblasFillMode_t  uplo,
                                            hipblasOperation_t transA,
                                            hipblasDiagType_t  diag,
                                            int                m,
                                            int                n,
                                            const float*       alpha,
                                            const float*       AP,
                                            int                lda,
                                            float*             BP,
                                            int                ldb);

The same goes for rocBLAS analogues rocblas_strmm, rocblas_dtrmm, rocblas_ctrmm, rocblas_ztrmm (ROCm/rocBLAS#1265).

So, the above 4 hipBLAS and 4 rocBLAS functions are marked as HIP UNSUPPORTED.

[Solution]
As far as hipBLAS doesn't support v1 BLAS functions, populate hipblas TRMM functions with two missing arguments: float* C and int ldc and revise functions' logic.

emankov added a commit to emankov/HIPIFY that referenced this issue Sep 19, 2022
+ Added tests for SYRK, SYMM, HEMM, and TRSM v2 functions
+ Added tests for HERKX (eXtended HERK) and GEAM functions

[ToDo]
+ Mark TRMM functions as UNSUPPORTED (both in `hip` and `roc`) till ROCm/hipBLAS#524 is fixed
+ File an analogous bug issue to rocBLAS
@daineAMD daineAMD self-assigned this Sep 19, 2022
@daineAMD
Copy link
Contributor

Hi @emankov,

I've made some comments in ROCm/rocBLAS#1265 regarding the rocBLAS implementation of these functions. The reasoning behind the difference for hipBLAS/cuBLAS is the same.

Moving forward, we do plan on moving to introduce the out-of-place functionality support (with the extra C and ldc parameters) for the hipblasXtrmm API. We announced deprecation of the current trmm API in hipBLAS 0.49 for ROCm 5.0. There is an open pull reqest at #504 which makes these requested changes. We plan on pushing this change into a future release of hipBLAS.

Thanks again,
Daine

emankov added a commit to emankov/HIPIFY that referenced this issue Sep 22, 2022
+ Mark rocBLAS TRMM functions rocblas_(s|d|c|z)trmm_outofplace, as supported only for TRMM v2 CUDA analogues
+ Mark hipBLAS TRMM functions hipblas(S|D|C|Z)trmm as HIP_UNSUPPORTED
+ Regenerate and update docs and hipify-perl accordingly

[Reasons]
+ hipBLAS TRMM functions hipblas(S|D|C|Z)trmm, actually, do not match neither cublas TRMM functions, nor cublas TRMM _v2 functions: ROCm/hipBLAS#524
+ There is a correspondence between cuBLAS cublas_(s|d|c|z)trmm and rocBLAS TRMM rocblas_(s|d|c|z)trmm_outofplace, not rocblas_(s|d|c|z)trmm: fixed it

[ToDo]
+ Close ROCm/rocBLAS#1265 as erroneous
+ Remove HIP_UNSUPPORTED mark from hipblas(S|D|C|Z)trmm functions after merging ROCm/hipBLAS#504
+ Add cublas2rocblas and update cublas2hipblas synthetic tests
@emankov
Copy link
Author

emankov commented Sep 22, 2022

Thank you @daineAMD,

I mentioned the following on my side (#640):
"Remove HIP_UNSUPPORTED mark from hipblas(S|D|C|Z)trmm functions after merging #504"

So, do you mind closing #524 after merging #504?

@daineAMD
Copy link
Contributor

Yes, I'll make sure to close #524 once #504 has been merged.

Thanks,
Daine

@emankov emankov changed the title [5.2.X] TRMM functions do not have correct correspondence in hipBLAS [5.3.X] TRMM functions do not have correct correspondence in hipBLAS Nov 7, 2022
@daineAMD
Copy link
Contributor

Hi @emankov,
The trmm interfaces have been updated in 215876e9 via #617. This commit is in the develop branch and should make it into the next release after ROCm 5.7. Please don't hesitate to reach out if you have further questions or concerns regarding this.

Thanks,
Daine

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

No branches or pull requests

2 participants