Skip to content

[SYCL][ABI-break] Remove redundant methods under sycl::handler #6461

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.

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

Closed
wants to merge 39 commits into from

Conversation

raaiq1
Copy link
Contributor

@raaiq1 raaiq1 commented Jul 21, 2022

Removed the following redundant methods:

  • void extractArgsAndReqsFromLambda(char *LambdaPtr, size_t KernelArgsNum,
    const detail::kernel_param_desc_t *KernelArgs);

  • void processArg(void *Ptr, const detail::kernel_param_kind_t &Kind,
    const int Size, const size_t Index, size_t &IndexShift,
    bool IsKernelCreatedFromSource);

  • static id<I> getDelinearizedIndex(const range<I> Range, const size_t Index);
    where I is could be {1,2,3};

@raaiq1 raaiq1 requested a review from a team as a code owner July 21, 2022 16:02
@raaiq1 raaiq1 requested a review from aelovikov-intel July 21, 2022 16:02
Signed-off-by: Rauf, Rana <[email protected]>
Copy link
Contributor

@aelovikov-intel aelovikov-intel left a comment

Choose a reason for hiding this comment

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

Looks reasonable, but I don't have enough experience to see if it's "complete".

Anyway, it leaves the code in a better state than before, so LGTM.

@bader bader changed the title [SYCL][ABI-break] Removed redundant methods under sycl::handler [SYCL][ABI-break] Remove redundant methods under sycl::handler Jul 26, 2022
@cperkinsintel cperkinsintel self-requested a review July 27, 2022 18:43
cperkinsintel
cperkinsintel previously approved these changes Jul 27, 2022
Copy link
Contributor

@cperkinsintel cperkinsintel left a comment

Choose a reason for hiding this comment

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

LGTM

@keryell
Copy link
Contributor

keryell commented Jul 28, 2022

Minor style wording: "methods" are named "member functions" in C++.

@raaiq1
Copy link
Contributor Author

raaiq1 commented Jul 28, 2022

Ohh, yeah, sorry. Still stuck in Java terms

@raaiq1
Copy link
Contributor Author

raaiq1 commented Jul 29, 2022

@intel/llvm-gatekeepers slight ping to merge commit. Thanks

1 similar comment
@raaiq1
Copy link
Contributor Author

raaiq1 commented Jul 29, 2022

@intel/llvm-gatekeepers slight ping to merge commit. Thanks

@pvchupin
Copy link
Contributor

pvchupin commented Aug 4, 2022

@raaiq1, can you resolve a conflict please?

@raaiq1 raaiq1 dismissed stale reviews from cperkinsintel and aelovikov-intel via f78c01d August 4, 2022 12:16
[SYCL][ABI-Break] Remove obsolete methods from memory_manager (intel#6522)
@raaiq1 raaiq1 marked this pull request as draft August 4, 2022 12:18
Signed-off-by: Rauf, Rana <[email protected]>
@raaiq1 raaiq1 marked this pull request as ready for review August 4, 2022 14:03
aelovikov-intel
aelovikov-intel previously approved these changes Aug 4, 2022
aelovikov-intel and others added 9 commits August 4, 2022 09:10
Previous patch was implemented by splitting off piece of a bigger patch by
completely eliminating "cl" namespace and then addressing the local failures.
Local testing didn't cover all possible platforms so these occurrences were left
untouched.

This is a wider application of grep/sed, but it's still not complete as some
instances of "cl" namespace references cannot be eliminated in an NFC
change (e.g. everything affecting/affected by mangling as in clang or some
tools). The reason I'm committing these changes separately is to ease the review
of the actual non-NFC PR later.
When _MT and _DLL are both defined, the user has passed /MD or /MDd and
is dynamically linking in the runtimes. Because std::cout etc are data,
they need to be marked dllimport in that case or else the linker cannot
find them.
Uplift GPU RT version for Linux to 22.29.23750

Co-authored-by: GitHub Actions <[email protected]>
Uplift GPU RT version for Linux to 22.30.23789

Co-authored-by: GitHub Actions <[email protected]>
Signed-off-by: Rauf, Rana <[email protected]>
…#6519)

Support for new FPGA attribute called [[intel::fpga_pipeline(N)]]
on intel#6254 is no longer needed.

This patch removes the support from frontend.

Signed-off-by: Soumi Manna <[email protected]>
Signed-off-by: Rauf, Rana <[email protected]>
@raaiq1 raaiq1 marked this pull request as draft August 4, 2022 17:40
@raaiq1
Copy link
Contributor Author

raaiq1 commented Aug 4, 2022

Alternate PR at: #6529

@raaiq1 raaiq1 closed this Aug 4, 2022
@raaiq1 raaiq1 deleted the handler_issue branch August 9, 2022 17:40
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