Reduce duplication within debug code #464
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
qjojo
approved these changes
Aug 31, 2022
jzuniga-amd
approved these changes
Sep 2, 2022
Prior to fmt 9.0.0, the inclusion of fmt/ostream.h would cause fmt to automatically use an existing ostream function to format the type. However, this would cause subtle issues during compilation if fmt/ostream.h were only sometimes included (as the definition of the formatting would change between files, in violation of the one-definition rule). As long as rocSOLVER supports the use of fmt 7.x and 8.x and uses fmt/ostream.h in any file, rocSOLVER should include fmt/ostream.h in every file that includes another fmt header.
This matches the existing default when building with install.sh or rmake.py. The difference was intended to provide different users with the defaults they prefer, but in practice it mostly caused confusion. Experience has shown that the defaults for directly using CMake and for building with the user of the helper script should be as similar as possible.
There is a clients/rocblascommon/utility.hpp and a library/rocblascommon/utility.hpp and the identical names cause confusion. In particular, common_host_helpers.hpp includes utility.hpp, which results in including a different file when the host helpers are used in the library vs. when the host helpers are used in the clients.
A header in common/ cannot depend on a header in library/, but common_host_helpers.hpp does. It only ever compiled because there were two different headers called utility.hpp. This is solved by moving the library/ utility.hpp to common/ and renaming it to reflect that it is used for rocblas types.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change reduces duplication in the common host helpers by providing a fmt printer for
rocblas_float_complexandrocblas_double_complex. If you have low-level functions specialized appropriately for both real and complex (likeformatorabsare), the higher level functions (likeprint_to_streamandprint_host_matrix) don't need to be specialized themselves.There were a couple fmtlib-related problems that I fixed at the same time.
rocblascommon/utility.hppandcommon/include/common_host_helpers.hppwas therefore including a different one when it was used in the library than when it was used in the client. This resulted in some very confusing behaviour. I've moved/renamed both headers and trimmed some unused stuff.This whole thing is basically just me cleaning up some of the current debug code before submitting another PR with some additions.