-
Notifications
You must be signed in to change notification settings - Fork 734
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
[Opaque Pointer] Fix bad code within INTEL_SYCL_OPAQUEPOINTER_READY #10870
Conversation
fa84874 Added the cast , we should keep it in opaque pointer path.
There was a problem hiding this 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
Thanks @MrSidims !
We have intel/dpcpp-tools-reviewers as code owner of ALL llvm changes. Is this intended? If not, we probably should update it.
|
Co-authored-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
|
||
uint64_t VTableIndex = CGM.getItaniumVTableContext().getMethodVTableIndex(GD); | ||
llvm::Value *VFunc; | ||
if (CGF.ShouldEmitVTableTypeCheckedLoad(MethodDecl->getParent())) { | ||
VFunc = CGF.EmitVTableTypeCheckedLoad( | ||
#ifdef INTEL_SYCL_OPAQUEPOINTER_READY | ||
MethodDecl->getParent(), VTable, PtrTy, | ||
#else |
There was a problem hiding this comment.
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.
@intel/llvm-gatekeepers Can we get this merged? Thanks! |
This is in preparation for setting -DINTEL_SYCL_OPAQUEPOINTER_READY=1