Conversation
alextmagro
requested review from
ipanfilo,
wangye805 and
wenchenvincent
as code owners
alextmagro
force-pushed
the
mxfp8_cast_optimization
branch
from
630d87a to
74a82fc
Compare
|
|
||
| #include "benchmark_utils.h" | ||
|
|
||
| #include "amd_detail/hip_float8.h" |
|
|
||
| #include "amd_detail/hip_float8.h" | ||
|
|
||
| #include <transformer_engine/cast_hip.h> |
There was a problem hiding this comment.
It is written in hip, but references the hipified headers in te. Would you prefer we write this in CUDA then hipify it?
benchmarks/cpp/utils/test_common.cpp
Outdated
| @@ -0,0 +1,1241 @@ | |||
| // !!! This is a file automatically generated by hipify!!! | |||
There was a problem hiding this comment.
Please add original file, not hipified
There was a problem hiding this comment.
Whoops. This file was copied over from the test/cpp directory to use the tensor capabilities, but forgot to clean it up to just keep what we need for the benchmarks. I will work on this, and push in a later commit.
There was a problem hiding this comment.
Regarding this, made a design change that's hopefully cleaner. I use the test_common files directly, but added a macro that ensures we only compile what we need within the executable.
transformer_engine/common/common.h
Outdated
| } | ||
|
|
||
| #define TRANSFORMER_ENGINE_CHUNK_DIM_SWITCH(use_large, CHUNK_Y, CHUNK_X, THREADS, ...) \ | ||
| [&] { \ |
There was a problem hiding this comment.
Those values are selected for specific kernel, aren't they? The define can bereplaced with TRANSFORMER_ENGINE_SWITCH_CONDITION
There was a problem hiding this comment.
Good idea, will make that change.
benchmarks/cpp/CMakeLists.txt
Outdated
| -DNDEBUG | ||
| -DUSE_ROCM | ||
| --offload-arch=${GPU_TARGETS} | ||
| -w |
There was a problem hiding this comment.
during debugging, I found the warnings to be clutter, especially since they came from TE rather than these benchmarks. I have removed the -w now that things are working.
benchmarks/cpp/run_benchmarks.sh
Outdated
| RED='\033[0;31m' | ||
| GREEN='\033[0;32m' | ||
| YELLOW='\033[1;33m' |
There was a problem hiding this comment.
I'd recommend not using colors here, these might not show up well in e.g. CI logs.
There was a problem hiding this comment.
Good point, will remove.
|
|
||
| if [ ! -f "${SCRIPT_DIR}/${test_common_hip}" ] || [ ! -f "${SCRIPT_DIR}/${test_common_h}" ]; then | ||
| echo -e "${RED}Error: hipified test_common files not found. Build tests before running benchmarks." | ||
| echo -e "Error: hipified test_common files not found. Build tests before running benchmarks." |
There was a problem hiding this comment.
Many of these echos don't need -e anymore. I'd also suggest removing the non-ASCII symbols like "✓" and "✗" - these don't always render well in terminals.
3 participants
Description
Improves performance of HIP MXFP8 Cast Kernels -- see confluence page for details
Adds benchmarking scripts for reproducability