remove options --no-enumerate #966
Conversation
Change package description to ROCm [ROCm/hipBLAS commit: b6c67fa]
NaveenElumalaiAMD
changed the title
|
Looks like the other changes to use cmake -E env also broke the build with old cmake versions. See #1006 but let us deal with that separately. Anyone on tensile team can say why --no-enumerate gave us a corrupt build? |
TorreZuk
left a comment
TorreZuk
left a comment
There was a problem hiding this comment.
We need a tensile team member to approve and hopefully comment
remove options --no-enumerate
|
--no-enumerate is not an impactful flag, this shouldn't affect the build. Safe to merge. |
Well @bstefanuk actually it was impactful as it led to serious performance degradations in the resulting build, thus we hoped for an explanation. It must be some side effect bug. |
This and the other comments about --no-enumerate sound serious. Is there some additional root causing needed here? |
| # We do not need to do device enumeration at library build time. | ||
| set(Options ${Options} "--no-enumerate") |
There was a problem hiding this comment.
As the comment says, there should be no need to enumerate devices at build time. Removing this line broke Windows builds in TheRock (ROCm/TheRock#1195 (comment)), where we don't yet build hipInfo.exe (ROCm/TheRock#600).
More root cause analysis is needed for this "reported performance regression". What actually changed, and why did removing this flag help? For benchmarks, are the libraries built on the same machines they are being run on? That could explain them expecting to be built with specific options, instead of using a more robust method of enumerating devices at test/benchmark time (on a test machine that might not be the same as the build machine).
SWDEV-531400 - Remove File reorganization backward computability (rocSOLVER)
This reverts commit 68a380c. This breaks building rocBLAS on Windows.
This reverts commit 68a380c. This breaks building rocBLAS on Windows.
Let's please revert this patch. The root cause makes no sense and the lack of any reproduction information makes it just lore that we will carry forward indefinitely (and if true, could be hiding a serious problem). |
This reverts commit 68a380c. #966 (comment)
This reverts commit 68a380c. This breaks building rocBLAS on Windows.
SWDEV-531400 - Remove File reorganization backward computability (rocSOLVER) [ROCm/rocSOLVER commit: 7d3e50c]
This reverts commit 68a380c. This breaks building rocBLAS on Windows.
This reverts commit 68a380c. This breaks building rocBLAS on Windows.
This reverts commit 68a380c. This breaks building rocBLAS on Windows.
This reverts commit 68a380c. This breaks building rocBLAS on Windows.
This reverts commit 68a380c. This breaks building rocBLAS on Windows.
… (#207)"
This reverts a line in the commit b5b297e
--no-enumerate, because of reported performance regression from the staging team.