Skip to content

[SYCL] Fix post commit fails. #6875

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 26, 2022
Merged

[SYCL] Fix post commit fails. #6875

merged 1 commit into from
Sep 26, 2022

Conversation

romanovvlad
Copy link
Contributor

  1. Updated checks to be more be more robust and work when clang discards names of values in the LLVM IR.
  2. Added a new symbol to the windows sybmols test.

@romanovvlad
[SYCL] Fix post commit fails.
8ad7b2f 
1. Updated checks to be more be more robust and work when clang discards
   names of values in the LLVM IR.
2. Added a new symbol to the windows sybmols test.
@romanovvlad romanovvlad requested a review from a team as a code owner September 25, 2022 20:07
@romanovvlad romanovvlad mentioned this pull request Sep 25, 2022
Merged
aelovikov-intel
aelovikov-intel reviewed Sep 26, 2022
View reviewed changes
sycl/test/basic_tests/accessor/host_acc_opt.cpp
Comment on lines 12 to 13
// CHECK-NOT: call
// CHECK-NOT: invoke
Copy link
Contributor

Choose a reason for hiding this comment

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

These aren't needed anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please clarify why?

Copy link
Contributor

Choose a reason for hiding this comment

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

Before the change lines 10-11 stopped working on line 12. Now line 12 doesn't exist so 10-11 are still in effect here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, will update in a separate PR. Thank you for noting.

aelovikov-intel
aelovikov-intel approved these changes Sep 26, 2022
View reviewed changes
Copy link
Contributor

@aelovikov-intel aelovikov-intel left a comment

Choose a reason for hiding this comment

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

I'm fine if the comment would be addressed in a separate PR to fix post-commit asap.

@romanovvlad romanovvlad merged commit 2f58f44 into intel:sycl Sep 26, 2022
@pvchupin
Copy link
Contributor

@romanovvlad, do you plan another PR? this one doesn't address the full problem.

@romanovvlad
Copy link
Contributor Author

@romanovvlad, do you plan another PR? this one doesn't address the full problem.

It's here: #6885

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

@aelovikov-intel aelovikov-intel aelovikov-intel approved these changes

@againull againull Awaiting requested review from againull againull is a code owner automatically assigned from intel/llvm-reviewers-runtime

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@romanovvlad @pvchupin @aelovikov-intel