Skip to content

[SYCL-MLIR] Mark helper functions static #11050

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 5, 2023
Merged

[SYCL-MLIR] Mark helper functions static #11050

merged 1 commit into from
Sep 5, 2023

Conversation

sommerlukas
Copy link
Contributor

Mark helper functions as static to avoid collision with functions in Polygeist by the same name.

@sommerlukas
[SYCL-MLIR] Mark helper functions static
ceb37f6 
Mark helper functions as static to avoid collision with functions in
Polygeist by the same name.
@sommerlukas sommerlukas added the sycl-mlir Pull requests or issues for sycl-mlir branch label Sep 4, 2023
@sommerlukas sommerlukas self-assigned this Sep 4, 2023
@sommerlukas
Copy link
Contributor Author

sommerlukas commented Sep 4, 2023 •
edited
Loading

The helper functions were introduced in this upstream patch: https://reviews.llvm.org/D154720

I think not marking them as static was simply an oversight in the upstream patch. Once approved here, I would also take this change upstream.

With this change, check-polygeist is working again if you use clang to compile the SYCL-MLIR project.

victor-eds
victor-eds approved these changes Sep 4, 2023
View reviewed changes
Copy link
Contributor

@victor-eds victor-eds left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

@victor-eds victor-eds added the upstream This change is related to upstreaming SYCL support to llorg. label Sep 4, 2023
@sommerlukas
Copy link
Contributor Author

We should not add the upstream tag anymore, it is now defined as "This change is related to upstreaming SYCL support to llorg" .

@sommerlukas sommerlukas removed the upstream This change is related to upstreaming SYCL support to llorg. label Sep 4, 2023
@victor-eds
Copy link
Contributor

We should not add the upstream tag anymore, it is now defined as "This change is related to upstreaming SYCL support to llorg" .

Oh, okey

@sommerlukas
Copy link
Contributor Author

Upstream PR was approved and merged, changes are identical to this PR: llvm/llvm-project#65303

@sommerlukas sommerlukas merged commit 2067948 into intel:sycl-mlir Sep 5, 2023
@sommerlukas sommerlukas deleted the mark-helpers-static branch September 5, 2023 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@victor-eds victor-eds victor-eds approved these changes

@whitneywhtsang whitneywhtsang whitneywhtsang approved these changes

@etiotto etiotto Awaiting requested review from etiotto

@Naghasan Naghasan Awaiting requested review from Naghasan

@jopperm jopperm Awaiting requested review from jopperm

Assignees

@sommerlukas sommerlukas

Labels
sycl-mlir Pull requests or issues for sycl-mlir branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sommerlukas @victor-eds @whitneywhtsang