Skip to content

Comments

[LLVM Test Suite] Adds run script for simple HIP tests#1973

Open
jplehr wants to merge 5 commits intoROCm:aomp-devfrom
jplehr:feat/enable-llvm-test-suite
Open

[LLVM Test Suite] Adds run script for simple HIP tests#1973
jplehr wants to merge 5 commits intoROCm:aomp-devfrom
jplehr:feat/enable-llvm-test-suite

Conversation

@jplehr
Copy link
Contributor

@jplehr jplehr commented Feb 18, 2026

Enables running the LLVM Test Suite External HIP simple tests.

The tests are a handful of simple HIP applications. We run these tests also in the upstream buildbots.
Eventually, we want to make this be able to run all of the HIP tests.

Copy link
Contributor

@Kewen12 Kewen12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

It would be helpful if we could document the usage of this script.

@jplehr
Copy link
Contributor Author

jplehr commented Feb 19, 2026

What sort of documentation would you like to see? A more general statement about what these run_* scripts are for or more in-depth explanation on each individual script?

I personally feel that the help text that this script produces is helpful enough, when you know what it is meant to achieve in general. But, of course, I cannot speak for others.

@Kewen12
Copy link
Contributor

Kewen12 commented Feb 20, 2026

What sort of documentation would you like to see? A more general statement about what these run_* scripts are for or more in-depth explanation on each individual script?

I personally feel that the help text that this script produces is helpful enough, when you know what it is meant to achieve in general. But, of course, I cannot speak for others.

Just a general guidance about this script? like what it does or when people should run it, etc.

Copy link
Contributor

@mhalk mhalk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Only a few nits.

echo "Using AOMP installation for HIP tests: ${ROCM_FOR_TESTS}"
else
# Fallback to system ROCm if AOMP doesn't have HIP libraries
: "${ROCM:=/opt/rocm}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Would have expected this at the top, with the other defaults.
(If yes, I'd also adapt the help message to actually print that var.)

Comment on lines +125 to +127
if [ ! -d "${LLVMTS_EXTERNAL_DIR}" ]; then
mkdir -p "${LLVMTS_EXTERNAL_DIR}"
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This is redundant. mkdir -p "${LLVMTS_EXTERNAL_DIR}/hip" is performed later and there's no check in-between.
Remove?

mkdir -p "${LLVMTS_EXTERNAL_DIR}"
fi

# Create hip subdirectory and symlink to ROCm installation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: What symlink? I guess ${ROCM_LINK} -> ${ROCM_FOR_TESTS}
Maybe move directly above the symlink creation? There's no usage until that or success check anyways.

ROCM_VERSION_FILE=$(dpkg -l rocm-core | grep rocm-core | awk '{print $3}' | cut -d- -f1)
else
# Default fallback version
ROCM_VERSION_FILE="6.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Would have expected this at the top, with the other defaults.
But on the other hand this behaves differently from : "${ROCM_VERSION_FILE:=6.0.0}"

@mhalk
Copy link
Contributor

mhalk commented Feb 20, 2026

What sort of documentation would you like to see? A more general statement about what these run_* scripts are for or more in-depth explanation on each individual script?
I personally feel that the help text that this script produces is helpful enough, when you know what it is meant to achieve in general. But, of course, I cannot speak for others.

Just a general guidance about this script? like what it does or when people should run it, etc.

That makes sense to me, but we need to be careful how specific that information is.
The more specific, the faster it might degrade, i.e. become inapplicable.

Currently, I would not exactly know how to use this and e.g. consult: https://llvm.org/docs/TestSuiteGuide.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants