Skip to content

[DeviceSanitizer] refine option parsing #15293

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 5 commits into from
Sep 25, 2024
Merged

[DeviceSanitizer] refine option parsing #15293

merged 5 commits into from
Sep 25, 2024

Conversation

@yingcong-wu
Copy link
Contributor

@yingcong-wu yingcong-wu commented Sep 5, 2024

Add a test for option parsing.

UR PR: oneapi-src/unified-runtime#2054

@yingcong-wu yingcong-wu mentioned this pull request Sep 5, 2024
Merged
@yingcong-wu yingcong-wu marked this pull request as ready for review September 9, 2024 01:51
@yingcong-wu yingcong-wu requested review from a team as code owners September 9, 2024 01:51
@yingcong-wu
Copy link
Contributor Author

yingcong-wu commented Sep 23, 2024

Kindly ping @dpcpp-sanitizers-review @intel/unified-runtime-reviewers for review.

aarongreig
aarongreig reviewed Sep 23, 2024
View reviewed changes
@yingcong-wu
Copy link
Contributor Author

yingcong-wu commented Sep 24, 2024

Kindly ping @intel/dpcpp-sanitizers-review for review.

AllanZyne
AllanZyne approved these changes Sep 24, 2024
View reviewed changes
Copy link
Contributor

@AllanZyne AllanZyne left a comment

Choose a reason for hiding this comment

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

Great!

zhaomaosu
zhaomaosu reviewed Sep 24, 2024
View reviewed changes
@yingcong-wu
Copy link
Contributor Author

yingcong-wu commented Sep 25, 2024

Hi @intel/llvm-gatekeepers , please help merge this PR, thanks.

@martygrant martygrant merged commit 340e133 into intel:sycl Sep 25, 2024
12 checks passed
@sarnex
Copy link
Contributor

sarnex commented Sep 25, 2024 •
edited
Loading

@yingcong-wu @intel/dpcpp-sanitizers-review Seeing this fail in postcommit on Arc, can someone take a look?

********************
Failed Tests (1):
  SYCL :: AddressSanitizer/common/option-redzone-size.cpp

https://github.com/intel/llvm/actions/runs/11037560969/job/30659164525

@yingcong-wu
Copy link
Contributor Author

yingcong-wu commented Sep 26, 2024

@sarnex Sure, I will take a look at this failure now.

@yingcong-wu yingcong-wu mentioned this pull request Sep 26, 2024
Merged
@yingcong-wu
Copy link
Contributor Author

yingcong-wu commented Sep 26, 2024

There is a FileCheck line within test case not updated accordingly, open PR #15517 to fix it.

steffenlarsen pushed a commit that referenced this pull request Sep 27, 2024
@yingcong-wu
[SYCL][E2E][DeviceSanitizer] update filename in the FileCheck line (#…
97b11a7 
…15517)

The filename is not updated accordingly with PR
#15293, this PR fixs this problem.
@yingcong-wu
Copy link
Contributor Author

yingcong-wu commented Sep 27, 2024

Hi @sarnex , a fix has been merged for the failure. Please let me know if there still problems. Thank you.

@sarnex
Copy link
Contributor

sarnex commented Sep 27, 2024

@yingcong-wu Seems fixed now, thanks!

@yingcong-wu yingcong-wu deleted the yc-0903-handle-option-exception branch October 28, 2024 02:43
AlexeySachkov
AlexeySachkov reviewed Feb 26, 2025
View reviewed changes
sycl/test-e2e/AddressSanitizer/common/options-invalid.cpp
@@ -0,0 +1,39 @@
// REQUIRES: linux
Copy link
Contributor

@AlexeySachkov AlexeySachkov Feb 26, 2025

Choose a reason for hiding this comment

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

This is an incorrect location for this test. Functionality that is being tested resides in Unified Runtime and therefore it should be tested there

@yingcong-wu yingcong-wu mentioned this pull request Feb 27, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexeySachkov AlexeySachkov AlexeySachkov left review comments

@zhaomaosu zhaomaosu zhaomaosu left review comments

@AllanZyne AllanZyne AllanZyne approved these changes

+1 more reviewer

@aarongreig aarongreig aarongreig approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@yingcong-wu @sarnex @AllanZyne @AlexeySachkov @aarongreig @zhaomaosu @martygrant