Skip to content

Conversation

@jsji
Copy link
Contributor

@jsji jsji commented Aug 17, 2023

This is in preparation for setting -DINTEL_SYCL_OPAQUEPOINTER_READY=1

  • Fix bad guard for INTEL_SYCL_OPAQUEPOINTER_READY
  • Keep the addressspace cast for opaque pointer
  • Fix opaque pointer guard for getVirtualFunctionPointer

@jsji jsji requested review from a team as code owners August 17, 2023 23:51
@jsji jsji requested review from bader and cdai2 August 17, 2023 23:52
@jsji jsji self-assigned this Aug 17, 2023
@jsji
Copy link
Contributor Author

jsji commented Aug 17, 2023

@cdai2 Please have a look at ce2ff56 , as this is corresponding changes in opaque pointer patch for OpenCL.

@jsji jsji closed this Aug 18, 2023
@jsji jsji reopened this Aug 18, 2023
@jsji
Copy link
Contributor Author

jsji commented Aug 18, 2023

Ping @intel/dpcpp-cfe-reviewers @intel/dpcpp-tools-reviewers @cdai2

The changes also tested with -DINTEL_SYCL_OPAQUEPOINTER_READY=1 in #10888

Copy link
Contributor

@MrSidims MrSidims left a comment

Choose a reason for hiding this comment

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

not sure, why tools team is summoned as code owners here, files are quite out of scope of our expertise
yet the changes look legit

@jsji
Copy link
Contributor Author

jsji commented Aug 21, 2023

Thanks @MrSidims !

not sure, why tools team is summoned as code owners here, files are quite out of scope of our expertise yet the changes look legit

We have intel/dpcpp-tools-reviewers as code owner of ALL llvm changes. Is this intended? If not, we probably should update it.

# DPC++ tools
llvm/ @intel/dpcpp-tools-reviewers

Co-authored-by: Mariya Podchishchaeva <[email protected]>
@jsji jsji requested review from Fznamznon and premanandrao August 21, 2023 17:10
VFunc = CGF.EmitVTableTypeCheckedLoad(
#ifdef INTEL_SYCL_OPAQUEPOINTER_READY
MethodDecl->getParent(), VTable, PtrTy,
#else
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like to me that we can avoid this #ifdef if we do the rename of TyPtr to PtrTy.
Other than this, we have to keep all the other #ifdef anyway, due to call to ->getPoitnerTo() , BitCast etc.

@jsji
Copy link
Contributor Author

jsji commented Aug 22, 2023

@intel/llvm-gatekeepers Can we get this merged? Thanks!

@steffenlarsen steffenlarsen merged commit f727cee into intel:sycl Aug 22, 2023
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.

5 participants