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

Revert "[SYCL] XFAIL LIT test due to duplicate diagnostic" #1460

Merged
merged 1 commit into from
Apr 30, 2020
Merged

Revert "[SYCL] XFAIL LIT test due to duplicate diagnostic" #1460

merged 1 commit into from
Apr 30, 2020

Conversation

premanandrao
Copy link
Contributor

This reverts commit a28778c
and fixes the test.

Signed-off-by: Premanand M Rao premanand.m.rao@intel.com

@premanandrao
Revert "[SYCL] XFAIL LIT test due to duplicate diagnostic"
d26c1c9 
This reverts commit a28778c
and fixes the test.

Signed-off-by: Premanand M Rao <premanand.m.rao@intel.com>
@premanandrao premanandrao requested a review from vladimirlaz April 2, 2020 16:59
premanandrao referenced this pull request Apr 2, 2020
@vladimirlaz
[SYCL] XFAIL LIT test due to duplicate diagnostic
a28778c 
Signed-off-by: Vladimir Lazarev <vladimir.lazarev@intel.com>
Fznamznon
Fznamznon reviewed Apr 3, 2020
View reviewed changes
Copy link
Contributor

@Fznamznon Fznamznon left a comment

Choose a reason for hiding this comment

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

Do you know what is the reason for duplicated diagnostics?
Why it didn't reproduce before new deferred diagnostics?

@premanandrao
Copy link
Contributor Author

Do you know what is the reason for duplicated diagnostics?

Basically what they are trying to solve with: https://reviews.llvm.org/D77028

Why it didn't reproduce before new deferred diagnostics?

We had a densemap of each caller-callee which had its diagnostic deferred. That prevented the second visit (at least in this case). We had other issues with duplicate messages that you had worked on.

@bader
Copy link
Contributor

bader commented Apr 3, 2020

Do you know what is the reason for duplicated diagnostics?

Basically what they are trying to solve with: https://reviews.llvm.org/D77028

@premanandrao, the test is already marked as expected to fail, so could we wait until the fix lands into the llvm-project and pull it into sycl branch, rather than update the test with wrong checks?

@premanandrao
Copy link
Contributor Author

@premanandrao, the test is already marked as expected to fail, so could we wait until the fix lands into the llvm-project and pull it into sycl branch, rather than update the test with wrong checks?

We could if that is what you prefer. You had asked @vladimirlaz to revert marking the test as "XFAIL", which is what I trying to help with.

@bader
Copy link
Contributor

bader commented Apr 3, 2020

Sorry for confusion. I draw Vladimir's attention that his approach to fix the test is a temporal workaround, which we should revert asap and replace with proper solution.
Now we know that this issue is caused by the community change and they work on a fix, so I think we can wait for community fix rather than fixing ourselves.

@vladimirlaz
Copy link
Contributor

vladimirlaz commented Apr 30, 2020 •
edited
Loading

it looks like https://reviews.llvm.org/D77028 was submitted but it doe not resolve the case.
I suggest to submit the change as it looks like permanent behavior.

@bader bader requested a review from erichkeane April 30, 2020 11:35
@bader bader merged commit c8d86a8 into intel:sycl Apr 30, 2020
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request May 6, 2020
Merge remote-tracking branch 'origin/sycl' into private/abatashe/user…
b952a76 
…_docs

* origin/sycl: (6482 commits)
  [SYCL][NFC] Clean formatting in Markdown documents (intel#1635)
  [SYCL][Doc] Remove obsolete parens from README (intel#1637)
  [SYCL] Fix failing ABI tests when LLVM_LIBDIR_SUFFIX is set (intel#1605)
  [SYCL] Fix warnings in libdevice (intel#1630)
  [SYCL][CUDA] Triage and clean LIT (intel#1620)
  [SYCL][NFC] Fix GCC 8 compilation warnings (intel#1631)
  [SYCL] Minor fixes in LowerWGScope
  [SYCL] PI: correct default interoperability plugin selection
  [SYCL] Add faster reduction implementations using atomic or/and intel::reduce() (intel#1615)
  [SYCL] Add sycl-ls utility for listing devices discovered/selected by SYCL RT (intel#1575)
  [SYCL] Fix getDeviceFromHandler declarations (intel#1626)
  [SPIR-V] Correct/improve declaration of SPIR-V builtins (intel#1519)
  [SYCL][USM] Improve USM allocator test and fix improper behavior. (intel#1538)
  [SYCL] Fix failing ABI LITs (intel#1622)
  [SYCL] Add support for MSVC internal math functions in device library (intel#1441)
  [SYCL] Add runtime library versioning (intel#1604)
  [SYCL] Check weak symbols in ABI dumps (intel#1609)
  [NFC][SYCL] Improve kernel metadata test (intel#1610)
  Revert "[SYCL] XFAIL LIT test due to duplicate diagnostic" (intel#1460)
  [SYCL] Move the reduction command group funcs out of handler.hpp (intel#1602)
  ...
@AlexeySachkov AlexeySachkov mentioned this pull request Feb 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fznamznon Fznamznon Fznamznon left review comments

@vladimirlaz vladimirlaz vladimirlaz approved these changes

@erichkeane erichkeane erichkeane 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
@premanandrao @bader @vladimirlaz @erichkeane @Fznamznon