Skip to content

Conversation

@MrSidims
Copy link
Contributor

It's no longer needed after SYCLPropagateAspectsUsage pass.

It's no longer needed after SYCLPropagateAspectsUsage pass.

Signed-off-by: Sidorov, Dmitry <[email protected]>
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

SYCL test changes LGTM.

Signed-off-by: Sidorov, Dmitry <[email protected]>
Copy link
Contributor

@maarquitos14 maarquitos14 left a comment

Choose a reason for hiding this comment

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

LGTM.

Signed-off-by: Sidorov, Dmitry <[email protected]>
@MrSidims MrSidims requested a review from a team as a code owner March 31, 2025 16:43
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

Thanks!

@MrSidims MrSidims requested a review from a team April 1, 2025 13:06
@MrSidims
Copy link
Contributor Author

MrSidims commented Apr 1, 2025

@intel/llvm-gatekeepers I think we can merge the patch.

@aelovikov-intel
Copy link
Contributor

Is NFC tag usage aligned with LLVM programming guide here?

@MrSidims
Copy link
Contributor Author

MrSidims commented Apr 1, 2025

Is NFC tag usage aligned with LLVM programming guide here?

@aelovikov-intel I'm not sure if LLVM programming guide defines NFC. I appreciate if you provide a quote from it with the definition, so I could evaluate if the PR is NFC from LLVM programming guide's stand point.

@aelovikov-intel
Copy link
Contributor

Not an official doc, but from this thread (https://discourse.llvm.org/t/nfc-abuse/58479/24):

That’s my litmus test: I see NFC is an indication that no test changes and none are expected to be provided.

@MrSidims
Copy link
Contributor Author

MrSidims commented Apr 1, 2025

Not an official doc, but from this thread (https://discourse.llvm.org/t/nfc-abuse/58479/24):

@aelovikov-intel should we formalize the definition and add it in documentation at least for https://github.com/intel/llvm ?

@aelovikov-intel
Copy link
Contributor

Not an official doc, but from this thread (https://discourse.llvm.org/t/nfc-abuse/58479/24):

@aelovikov-intel should we formalize the definition and add it in documentation at least for https://github.com/intel/llvm ?

Up to you, I've just highlighted a possible confusion. If you think current title is fine, I don't mind.

@MrSidims MrSidims changed the title [NFC][SYCL] Remove srcloc metadata before sycl-post-link [NFCI][SYCL] Remove srcloc metadata before sycl-post-link Apr 1, 2025
@MrSidims
Copy link
Contributor Author

MrSidims commented Apr 1, 2025

Updated the tag to NFCI

@bader
Copy link
Contributor

bader commented Apr 3, 2025

Updated the tag to NFCI

@aelovikov-intel, please, merge PR if you have no other comments.

@aelovikov-intel aelovikov-intel merged commit 3a353b1 into intel:sycl Apr 3, 2025
22 of 23 checks passed
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