-
Notifications
You must be signed in to change notification settings - Fork 795
Description
Describe the bug
sycl/test-e2e/VirtualFunctions/multiple-translation-units/separate-call.cpp test that is added by #15067 fails with multiple definitions error, because we incorrectly copy vtables into every module we produce during the device code split.
To reproduce
That's an in-tree e2e test, it should be clear how to reproduce it.
Environment
- OS: Linux
- Target device and vendor: this should not be device-dependent at all, I used PVC to reproduce
- DPC++ version: recent intel/llvm build
- Dependencies version: doesn't matter here
Additional context
VTables may rightfully have external linkage if virtual functions from classes they are generated for have external linkage.
In sycl-post-link we copy every global variable that is non-discardable even if unused into every split we produce. Later, when we link together different device images like one that provides virtual functions and another which uses them, we see multiple definition of vtables.
A possible solution here is to annotate vtables during their LLVM IR codegen so that we can omit them from initial set of globals which we copy during device code split and only include them if they are actually used by a module in accordance with a dependency graph.
However, we need to be careful here, because it could be the case that two kernels create objects of the same class containing virtual functions and those kernels are outlined into separate device images. Both device images will use vtable, so it will be defined in both, but they will be later linked together and we will again encounter multiple definition error.
We could make vtables linkonce_odr so they do not cause multiple definitions error and can be discarded by some internalization optimizations from modules where they are not used.
Alternatively, we can only emit vtables into device images which define virtual functions, but since those can also be split into different device images (if virtual functions from the same class belong to different virtual function sets) and it won't be clear which device image to use in that case. Perhaps the approach with linkonce_odr is the most reliable here.