Skip to content
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

small fix required for SYCL/clang20 #784

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

flg
Copy link
Contributor

@flg flg commented Nov 26, 2024

Sycl/clang20 range index could no longer be convert to sycl::id so changed it to sycl::nd_item.
Not sure this is the best way to do it, happy to be advised by experts.

Error showed up with recent version of SYCL compiler:

$ clang++ -v
Intel(R) oneAPI DPC++/C++ Compiler 2025.0.1 (2025.0.1.20241113)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /opt/intel/oneapi/compiler/2025.0/bin/compiler
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/11
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/13
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/13
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Candidate multilib: x32;@mx32
Selected multilib: .;@m64
Found HIP installation: /opt/rocm, version 6.2.41134

from 24/11/2024 daily release of Intel's LLVM:
https://github.com/intel/llvm/releases/download/nightly-2024-11-24/sycl_linux.tar.gz

@stephenswat
Copy link
Member

I believe you may have made a git error here, as you seem to have included 29 commits. We should work on fixing this so only the necessary commits are included.

@flg
Copy link
Contributor Author

flg commented Nov 26, 2024

Ho yes this looks bad, sorry. Not familiar with workflow involving forks. Not sure how to fix this.

@stephenswat
Copy link
Member

stephenswat commented Nov 26, 2024

What seems to have happened is that your flg_syclclang20 branch branched off the traccc main branch when you committed f4080e4, and then you pulled in additional commits from main. The problem is then that you wrote 4b572aa on top of your branch, which was no longer in sync with the main branch.

To fix the issue, I recommend the following steps:

  1. Ensure you have a remote referring to the traccc upstream repository; if you don't, execute git remote add upstream https://github.com/acts-project/traccc.git
  2. Fetch the current traccc main branch; git fetch upstream
  3. Save your commit 4b572aa in a new branch: git checkout -b my_backup_branch 4b572aaf
  4. Go back to your flg_syclclang20 branch: git checkout flg_syclclang20
  5. Reset the branch to track main: git reset --hard upstream/main
  6. Cherry-pick the commit: git cherry-pick my_backup_branch
  7. Force push your updated branch: git push --force

That should do it.

Note: you can probably skip steps 3 and 4 and just cherry-pick 4b572aa directly, but it is technically possible for git to garbage collect your commit if nothing points at it.

@krasznaa
Copy link
Member

At the same time: With any branch that "somehow broke", you can always consider to just abandon that branch completely. Close the PR, and open a new branch PR instead. Either cherry-picking relevant commits from the "dodgy branch", or in the worst of cases possibly just cloning the repository twice, and then copying code from one set of files to the other set of files by hand. (I had to do the latter once or twice in the past, when doing things "correctly using git" would've been too much hassle.)

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@flg
Copy link
Contributor Author

flg commented Nov 26, 2024

@stephenswat Thanks a lot. It looks better now. Hope it's good this time.

@stephenswat
Copy link
Member

@flg this looks good, but could you format the code using clang-format? Then this will be ready to go as far as I am concerned.

@stephenswat stephenswat enabled auto-merge (squash) November 27, 2024 09:39
@stephenswat stephenswat merged commit c06c784 into acts-project:main Nov 27, 2024
21 checks passed
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