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.

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

[SYCL] BE is expecting FP accuracy values without the suffix "f". #10525

Merged
merged 2 commits into from
Jul 25, 2023
Merged

[SYCL] BE is expecting FP accuracy values without the suffix "f". #10525

merged 2 commits into from
Jul 25, 2023

Conversation

zahiraam
Copy link
Contributor

@zahiraam zahiraam commented Jul 21, 2023 •
edited
Loading

Currently we are passing values of the FP accuracy with an "f" suffix. The backend is expecting those values without the suffix. This patch removes them.

@zahiraam zahiraam temporarily deployed to aws July 21, 2023 16:01 — with GitHub Actions Inactive
@zahiraam zahiraam temporarily deployed to aws July 21, 2023 16:39 — with GitHub Actions Inactive
@zahiraam zahiraam temporarily deployed to aws July 21, 2023 18:37 — with GitHub Actions Inactive
@zahiraam zahiraam temporarily deployed to aws July 21, 2023 19:24 — with GitHub Actions Inactive
@zahiraam zahiraam changed the title [SYCL] BE is expecting FP accuracy values without the "f". [SYCL] BE is expecting FP accuracy values without the suffix "f". Jul 21, 2023
@zahiraam zahiraam marked this pull request as ready for review July 21, 2023 19:27
@zahiraam zahiraam requested review from a team as code owners July 21, 2023 19:27
premanandrao
premanandrao approved these changes Jul 24, 2023
View reviewed changes
Copy link
Contributor

@premanandrao premanandrao left a comment

Choose a reason for hiding this comment

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

FE test change looks good to me.

AlexeySachkov
AlexeySachkov approved these changes Jul 24, 2023
View reviewed changes
Copy link
Contributor

@AlexeySachkov AlexeySachkov left a comment

Choose a reason for hiding this comment

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

llvm/ changes LGTM

@zahiraam
Copy link
Contributor Author

zahiraam commented Jul 24, 2023 •
edited by Fznamznon
Loading

@intel/llvm-gatekeepers can you consider merging this please? Thanks.

asudarsa
asudarsa approved these changes Jul 25, 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.

LGTM. Thanks

@steffenlarsen steffenlarsen merged commit 5096c20 into intel:sycl Jul 25, 2023
16 checks passed
@zahiraam zahiraam mentioned this pull request Aug 1, 2023
Merged
mdtoguchi pushed a commit to mdtoguchi/llvm that referenced this pull request Oct 18, 2023
@zahiraam @mdtoguchi
[SYCL] BE is expecting FP accuracy values without the suffix "f". (in…
47b02f7 
…tel#10525)

Currently we are passing values of the FP accuracy with an "f" suffix.
The backend is expecting those values without the suffix. This patch
removes them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexeySachkov AlexeySachkov AlexeySachkov approved these changes

@premanandrao premanandrao premanandrao approved these changes

@asudarsa asudarsa asudarsa approved these changes

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

Successfully merging this pull request may close these issues.
5 participants
@zahiraam @AlexeySachkov @premanandrao @asudarsa @steffenlarsen