TRMM, remove deprecated inplace trmm to favor outofplace/inplace trmm API #617
Conversation
amcamd
commented
Jun 21, 2023
amcamd
commented
- this PR does 3 things
- removes the macro HIPBLAS_V1 and code used for deprecating trmm_inplace
- copies from daineAMD PR hipblasTrmm API change to allow inplace and outofplace behaviour #504 to switch from trmm_inplace to trmm_outofplace. This contains most of the changes
- call rocblas_Xtrmm in place of rocblas_Xtrmm_outofplace. This needs the changes in rocBLAS-internal PR # 1926
- This PR will fail tests untill rocBLAS-internal PR # 1926 is merged
amcamd
requested review from
TorreZuk,
mahmoodw,
daineAMD,
bragadeesh and
NaveenElumalaiAMD
as code owners
clients/samples/example_strmm.cpp
Outdated
| /* ************************************************************************ | ||
| * Copyright (C) 2016-2023 Advanced Micro Devices, Inc. All rights reserved. | ||
| * | ||
| * Permission is hereby granted, free of charge, to any person obtaining a copy |
There was a problem hiding this comment.
Why wasn't this just converted to show the new API use?
There was a problem hiding this comment.
Thanks for catching this. Added back example_strmm.cpp in next commit.
| #ifndef HIPBLAS_NO_DEPRECATED_WARNINGS | ||
| #define HIPBLAS_NO_DEPRECATED_WARNINGS | ||
| #endif |
There was a problem hiding this comment.
Maybe just comment out for next deprecation use?
| @@ -90,3 +90,49 @@ Packed int8x4 was removed as support for arbitrary dimensioned int8_t data is a | |||
| * enum hipblasInt8Datatype_t was removed | |||
| * function hipblasSetInt8Datatype was removed | |||
| * function hipblasGetInt8Datatype was removed | |||
|
|
|||
| Legacy BLAS in-place trmm | |||
There was a problem hiding this comment.
Are you adding another Removed in hipBLAS 2.0 section for the changes here?
There was a problem hiding this comment.
Thanks for finding this, added Removed in hipBLAS 2.0 section in next commit.
library/include/hipblas.h
Outdated
| Note that trmm can provide in-place functionality by passing in the same address for both | ||
| matrices B and C. | ||
|
|
There was a problem hiding this comment.
When B == C, rocBLAS requires that ldb == ldc, otherwise we return an error code. I think that cuBLAS doesn't do any of this checking, but would have to double check. Might be worth mentioning, or guide users to look at rocBLAS/cuBLAS documentation to avoid having to update this if backends ever change.
There was a problem hiding this comment.
Thanks for finding this, added requires ldb == ldc in next commit.
