Skip to content

[SYCL][HIP] Remove arch requirement for running lit tests #5253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 15, 2022

Conversation

npmiller
Copy link
Contributor

@npmiller npmiller commented Jan 3, 2022

This flag was introduced before all the tests that run code were moved
to the llvm-test-suite repository. The tests left in this repository
are only compiler tests so the offload architecture can simply be
hardcoded as the tests don't depend on what hardware is available.

This simplifies building and running the tests for hip.

@npmiller npmiller requested review from bader and a team as code owners January 3, 2022 15:53
@npmiller npmiller requested a review from steffenlarsen January 3, 2022 15:53
@npmiller
Copy link
Contributor Author

npmiller commented Jan 3, 2022

@bader I've updated it wherever I could including in .github but it looks like the CI is failing with:

configure.py: error: unrecognized arguments: --hip-amd-arch=gfx906

So there might be some things to update in the CI in your side.

If it's easier to test I can also split this patch into unwiring the flag and actually removing it. So we could run the CI with the flag but using the hardcoded path internally, then remove the flag from the CI, then remove the flag from the script. Just let me know.

@pvchupin pvchupin requested a review from alexbatashev January 4, 2022 17:15
alexbatashev
alexbatashev previously approved these changes Jan 4, 2022
Copy link
Contributor

@alexbatashev alexbatashev left a comment

Choose a reason for hiding this comment

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

LGTM, the failure for one of HIP jobs is expected

bader
bader previously approved these changes Jan 10, 2022
@npmiller npmiller dismissed stale reviews from bader and alexbatashev via fb08b31 January 10, 2022 11:56
bader
bader previously approved these changes Jan 11, 2022
@npmiller
Copy link
Contributor Author

npmiller commented Jan 31, 2022

@bader I'm unsure where the test failure is on here is coming from but I don't really think this patch could cause it, could we try re-running the CI?

@bader
Copy link
Contributor

bader commented Jan 31, 2022

@bader I'm unsure where the test failure is on here is coming from but I don't really think this patch could cause it, could we try re-running the CI?

https://github.com/intel/llvm/runs/4773548691?check_suite_focus=true - this looks suspicious, it might be that your change exposed a bug in the test.

To re-run the tests, I suggest pulling latest changes from the sycl branch.

npmiller and others added 5 commits January 31, 2022 11:32
This flag was introduced before all the tests that run code were moved
to the `llvm-test-suite` repository. The tests left in this repository
are only compiler tests so the offload architecture can simply be
hardcoded as the tests don't depend on what hardware is available.

This simplifies building and running the tests for hip.
Co-authored-by: Alexey Bader <[email protected]>
@npmiller
Copy link
Contributor Author

Rebased on top of the SYCL branch. Although now it's failing because it removes the buildbot flag, I'm not sure how these were run last time, they might need to be run manually.

@bader
Copy link
Contributor

bader commented Jan 31, 2022

Although now it's failing because it removes the buildbot flag, I'm not sure how these were run last time, they might need to be run manually.

The failure is expected GitHub Actions (GHA) runs workflow twice when GHA scripts are changed: one run uses old workflow scripts and another run uses new workflow scripts.
Logs with updated GHA script are clean - https://github.com/intel/llvm/runs/5005328456?check_suite_focus=true
Logs with outdated GHA script can be ignored - https://github.com/intel/llvm/runs/5005348172?check_suite_focus=true

@bader bader merged commit 1600218 into intel:sycl Feb 15, 2022
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Feb 23, 2022
…/llvm into refactor_existing_workflows

* 'refactor_existing_workflows' of github.com:alexbatashev/llvm: (2051 commits)
  [SYCL][L0] Honor property::queue::enable_profiling (intel#5543)
  [SYCL][CI] Enable sccache on Windows (intel#5589)
  [SYCL][Doc] Move internal design docs (intel#5556)
  [sycl-post-link] Initialize the integer Value variable (intel#5585)
  [CI] Fix nightly builds (intel#5584)
  [SYCL][L0] Fix use of copy-engines in L0 interop queue (intel#5579)
  Update OpenCL headers tag to dcd5bed (intel#5575)
  [SYCL] Fix warning for InOrderQueueSyncCheck unit test build (intel#5577)
  [SYCL][HIP] Remove arch requirement for running lit tests (intel#5253)
  [SYCL][L0] Fix timestamp calculation (in ns) (intel#5555)
  [SYCL] Fix sync of host task vs kernel for in-order queue (intel#5551)
  [sycl-post-link] Add a check for device globals with device_image_scope (intel#5517)
  [SYCL] Fix SYCL Kernel Body Check (intel#5546)
  [SYCL] Add support for SYCL 2020 in class group (intel#5447)
  Fix tests after 1c729d7 Use align attribute for kernel pointer arg alignment
  Fix tests after 18834dc Mark pointer-typed kernel arguments as ABI aligned
  [CI] Add experimental Windows build to GitHub Actions nightly (intel#5560)
  [sycl-post-link][NFC] Address clang-tidy concerns in the sycl-post-link (intel#5552)
  Fix lit test after changes in Clang
  Improve backward compatibility for DISubRange
  ...
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