Skip to content

Conversation

@srividya-sundaram
Copy link
Contributor

No description provided.

@srividya-sundaram srividya-sundaram requested a review from a team as a code owner November 22, 2022 05:39
@premanandrao
Copy link
Contributor

Could you also a CodeGen test for this to check if we do the right thing after we allow this through in Sema?

@smanna12
Copy link
Contributor

Could you also a CodeGen test for this to check if we do the right thing after we allow this through in Sema?

I agree with Prem.

@srividya-sundaram
Copy link
Contributor Author

I see the ESIMD and CUDA LLVM Test Suite failures in other PRs too. They don't seem to be caused by changes in this PR.

@srividya-sundaram
Copy link
Contributor Author

@intel/llvm-gatekeepers Hi, this is ready for merging. Thanks!

@steffenlarsen
Copy link
Contributor

Failures in CI are seen on other PRs (for example #7563) and have been reported in #6524 (comment).

@steffenlarsen steffenlarsen merged commit d8fd9bc into intel:sycl Nov 29, 2022
@pvchupin
Copy link
Contributor

@srividya-sundaram, can you please follow up on Windows failure in post-commit for the new test you've added: https://github.com/intel/llvm/actions/runs/3576029259/jobs/6013328053

@srividya-sundaram
Copy link
Contributor Author

@srividya-sundaram, can you please follow up on Windows failure in post-commit for the new test you've added: https://github.com/intel/llvm/actions/runs/3576029259/jobs/6013328053

Hi! Is there a way I can reproduce the post-commit failure locally on a Windows machine?
I do have a fix looking at the log file, but I want to make sure it does really fix the failure.

@premanandrao
Copy link
Contributor

Is there a way I can reproduce the post-commit failure locally on a Windows machine?

I think you should be able to run the command-line on a Windows local build.

@srividya-sundaram
Copy link
Contributor Author

@srividya-sundaram, can you please follow up on Windows failure in post-commit for the new test you've added: https://github.com/intel/llvm/actions/runs/3576029259/jobs/6013328053

@pvchupin Just curious. Would it have been possible to capture this failure before it got merged?
I am not aware of the details of the post-commit checks, but wondering if it would help to have such failures be captured sooner than later in the pipeline.

@premanandrao
Copy link
Contributor

Would it have been possible to capture this failure before it got merged? I am not aware of the details of the post-commit checks, but wondering if it would help to have such failures be captured sooner than later in the pipeline.

Does it not show up in a check-clang test run on Windows?

@pvchupin
Copy link
Contributor

@pvchupin Just curious. Would it have been possible to capture this failure before it got merged? I am not aware of the details of the post-commit checks, but wondering if it would help to have such failures be captured sooner than later in the pipeline.

We don't run windows in pre-commit (due to longer pre-commit time). So you could detect this only if run locally.
It's ok to address windows specific issues post-commit.

@elizabethandrews
Copy link
Contributor

Would it have been possible to capture this failure before it got merged? I am not aware of the details of the post-commit checks, but wondering if it would help to have such failures be captured sooner than later in the pipeline.

Does it not show up in a check-clang test run on Windows?

Yes it would but I very rarely test my patch on Windows because I work on Linux and I suspect its the same for most developers who work on Linux. I don't think we're expected to do this anyway.

@premanandrao
Copy link
Contributor

Yes it would but I very rarely test my patch on Windows because I work on Linux and I suspect its the same for most developers who work on Linux. I don't think we're expected to do this anyway.

No, I understand. I am that way too. I was worried initially that local runs on Windows were not showing the failure; but then I understood the question was more about the expected development/testing process.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants