Added run_HeCBench.sh to aomp/bin. - #2396
Conversation
mhalk
left a comment
mhalk
left a comment
There was a problem hiding this comment.
Generally: make sure this script is shellcheck-compliant.
IMHO the VERBOSE-handling makes the script much harder to read and constantly "breaks the flow".
| _AOMP_USER_SET=0 | ||
| _ROCM_USER_SET=0 | ||
| [ -n "${AOMP+x}" ] && _AOMP_USER_SET=1 | ||
| [ -n "${ROCM+x}" ] && _ROCM_USER_SET=1 |
There was a problem hiding this comment.
Sorry my fault, $ROCM_PATH is the canonical EnVar AFAICT, see:
https://rocm.docs.amd.com/en/latest/reference/environment-variables/index.html#hipcc-environment-variables
| if timeout "$HECBENCH_TIMEOUT" "${make_run[@]}" >>"$results" 2>&1; then | ||
| rc=0 | ||
| else | ||
| rc=$? | ||
| fi | ||
| if [ $rc -eq 0 ]; then | ||
| echo "STATUS $d: PASS" | tee -a "$results" | ||
| "${make_clean[@]}" >/dev/null 2>&1 | ||
| else | ||
| echo "STATUS $d: FAIL(rc=$rc)" | tee -a "$results" | ||
| fi |
There was a problem hiding this comment.
What's the reason that lines 164,165 are not folded into the if block in 159?
Same question for the else block in 167.
| if timeout "$HECBENCH_TIMEOUT" "${make_run[@]}" >>"$results" 2>&1; then | ||
| rc=0 | ||
| else | ||
| rc=$? | ||
| fi |
There was a problem hiding this comment.
What's the reason we cannot use $? directly for a successful run?
There was a problem hiding this comment.
The variable rc was not needed. We need to be careful with $?, since it changes with each invocation of a Linux utility.
mhalk
left a comment
mhalk
left a comment
There was a problem hiding this comment.
Did a thorough pass -- most of these are mere suggestions to improve style and readability.
Basically NFCI comments :)
| if timeout "$HECBENCH_TIMEOUT" "${make_run[@]}" >>"$results" 2>&1; then | ||
| rc=0 | ||
| else | ||
| rc=$? | ||
| fi | ||
| if [ $rc -eq 0 ]; then | ||
| echo "STATUS $d: PASS" | tee -a "$results" | ||
| "${make_clean[@]}" >/dev/null 2>&1 | ||
| else | ||
| echo "STATUS $d: FAIL(rc=$rc)" | tee -a "$results" | ||
| fi |
mhalk
left a comment
mhalk
left a comment
There was a problem hiding this comment.
LGTM! I think this is a solid base -- Thank you Lynd!
(Added another suggestion I simply forgot to send during the previous review.)
3 participants
Motivation
Allow developers to easily run HeCBench (a suite of benchmarks) with hip and aomp. They can run individual
benchmarks with run_HeCBench.sh.
Technical Details
Added run_HeCBench.sh