Skip to content

[Opaque Pointer][LLVM][Test] Remove ifdef and port tests #11062

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
aelovikov-intel merged 1 commit into from
Sep 8, 2023
Merged

[Opaque Pointer][LLVM][Test] Remove ifdef and port tests #11062

aelovikov-intel merged 1 commit into from
Sep 8, 2023

Conversation

@jsji
Copy link
Contributor

@jsji jsji commented Sep 5, 2023 •
edited
Loading

These tests were guarded within ifdef, so missed in last porting.

@jsji jsji requested a review from a team as a code owner September 5, 2023 13:09
@jsji jsji self-assigned this Sep 5, 2023
@jsji jsji closed this Sep 5, 2023
@jsji jsji reopened this Sep 5, 2023
@jsji jsji mentioned this pull request Sep 6, 2023
Merged
@jsji jsji changed the title [Opaque Pointer][LLVM] Remove ifdef and port tests" [Opaque Pointer][LLVM][Test] Remove ifdef and port tests Sep 6, 2023
@jsji
Copy link
Contributor Author

jsji commented Sep 6, 2023

Ping @intel/dpcpp-tools-reviewers . Thanks.

@jsji jsji requested a review from bader September 6, 2023 18:55
@jsji
Copy link
Contributor Author

jsji commented Sep 8, 2023

Thanks @bader ! @intel/llvm-gatekeepers Can we get this merged. Thanks!

@bader
Copy link
Contributor

bader commented Sep 8, 2023

@jsji, we need code owners' approval to merge this.

@jsji
Copy link
Contributor Author

jsji commented Sep 8, 2023

@jsji, we need code owners' approval to merge this.

Ah... Sure. Did not notice that @intel/dpcpp-tools-reviewers haven't reviewed. Thanks for reminding.

asudarsa
asudarsa reviewed Sep 8, 2023
View reviewed changes
llvm/test/Assembler/scalable-vector-struct.ll
@@ -1,23 +1,21 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
; ifndef INTEL_SYCL_OPAQUEPOINTER_READY
Copy link
Contributor

@asudarsa asudarsa Sep 8, 2023

Choose a reason for hiding this comment

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

I thought we remove the code inside ifndef. Can you please clarify?

Thanks

Copy link
Contributor Author

@jsji jsji Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, this was guarded because we were only testing typed pointer (the test can only be run in typed pointer mode). This PR ported the testcase into opaque pointer, so we now can remove the guard to let it run.

asudarsa
asudarsa approved these changes Sep 8, 2023
View reviewed changes
Copy link
Contributor

@asudarsa asudarsa left a comment

Choose a reason for hiding this comment

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

I think I understand the changes. Just requested a quick clarification. But the changes are fine.

Thanks

@jsji
Copy link
Contributor Author

jsji commented Sep 8, 2023

@intel/llvm-gatekeepers Can we get this merged. Thanks!

@aelovikov-intel aelovikov-intel merged commit 026babb into intel:sycl Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bader bader bader approved these changes

@asudarsa asudarsa asudarsa approved these changes

Assignees

@jsji jsji

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jsji @bader @asudarsa @aelovikov-intel