Revert "[sycl-web] Fit LIT tests failures in CodeGenSYCL. (#10820)"

[mikerice1969]

This reverts commit 66c18fe.

This restores the lost vtable type changes from upstream (8acdcf4).

[elizabethandrews]

Hi @mikerice1969. I just worked on the same patch. I realized these were missing when I was trying to port some of @jyu2-git's changes from downstream for Virtual functions. The changes I made however take into account whether the target isSpir (similar to how this was merged downstream). Is there a reason you did not do that here? I haven't updates lit tests in my patch and so am not 100% certain my changes are correct yet

[mikerice1969]

Hi @mikerice1969. I just worked on the same patch. I realized these were missing when I was trying to port some of @jyu2-git's changes from downstream for Virtual functions. The changes I made however take into account whether the target isSpir (similar to how this was merged downstream). Is there a reason you did not do that here? I haven't updates lit tests in my patch and so am not 100% certain my changes are correct yet

This just restores the virtual vtable pointer changes from upstream. If this doesn't make anything worse it's probably best to submit this to get back to upstream then make additional changes to improve virtual functions as needed. I'd like to see your patch since I don't think what we have downstream is necessarily the right way to fix the problem.

[elizabethandrews]

Hi @mikerice1969. I just worked on the same patch. I realized these were missing when I was trying to port some of @jyu2-git's changes from downstream for Virtual functions. The changes I made however take into account whether the target isSpir (similar to how this was merged downstream). Is there a reason you did not do that here? I haven't updates lit tests in my patch and so am not 100% certain my changes are correct yet

This just restores the virtual vtable pointer changes from upstream. If this doesn't make anything worse it's probably best to submit this to get back to upstream then make additional changes to improve virtual functions as needed. I'd like to see your patch since I don't think what we have downstream is necessarily the right way to fix the problem.

I'll upload a draft PR in a bit. But it is essentially the same thing as downstream merge.

I'd like to see your patch since I don't think what we have downstream is necessarily the right way to fix the problem.

I wasn't sure about this either. I will reach out to you offline to discuss this.

[elizabethandrews]

I spoke to @mikerice1969 offline and I agree with his sentiment that it may be best to merge this patch from community as-is and add customization if required in a follow-up.

[elizabethandrews]

@mikerice1969 I've uploaded 2 PRs (which are very much in progress with random comments I've made for my own debugging) in case you are interested.

#12788 ports changes made downstream to add address spaces to virtual table components. It includes the community changes you made in this PR albeit with downstream customizations. I felt my PR was very messy and confusing and so extracted the community commit changes to #12789, with downstream customizations. It is also a WIP. If we aren't sure the downstream customizations are correct, I will pause working on this till we discuss this further.

[mikerice1969]

Hi @intel/llvm-gatekeepers, please go ahead and merge this. We will follow-up with new PRs for additional work to improve virtual function support.