Fix common config for including STL headers before oneDPL on GCC9 and GCC10
There is an issue while STL headers before <oneapi/dpl/execution> on GCC9 and GCC10 with modern oneTBB versions:
#include <cstddef>
#include <oneapi/dpl/execution>
int main() { ... }
Any STL header includes the file bits/c++config.h that defines macros __PSTL_USE_PAR_POLICIES and __PSTL_PAR_BACKEND_TBB. Since STL headers goes before oneDPL/common_config.h, these macros would be defined to 1 because oneTBB is present but the version is not checked.
If we want to include <execution> safely in <oneapi/dpl/execution>, these macros should be unset in common config.
I do not have much to add, but I have installed GCC 9/10 and have confirmed that the following program does not compile on the main branch with gcc9/gcc10 and it now successfully compiles with this PR.
#include <cstddef>
#include <oneapi/dpl/execution>
int main() { return 0; }
I have the following understanding of the cases of handling incompatibility between TBB backend in libstdc++ v9/v10 and modern TBB, oneTBB, when the later is used:
- oneDPL headers are included first.
PSTL_USE_PARALLEL_POLICIESis set to0; when libstdc++ pulls in its configs, it sets internal macros (__PSTL_USE_PAR_POLICIESand__PSTL_PAR_BACKEND_TBB) according toPSTL_USE_PARALLEL_POLICIESand thus turn offs TBB backend. It disables the parallel policies entirely in libstdc++ v9 (because there is only TBB backend) and fall backs to the serial backend in libstdc++ v10. - oneDPL headers are included after an STL header from libstdc++ (e.g.
execution). It is too late, nothing can be done here. - oneDPL headers are included after a non-STL header from libstdc++ (e.g.
cstddef). We can fix it, what this PR does. At this point, libstdc++ has already set the internal macros to use TBB, but it has not started using it. The PR just redefines these internal macros, so TBB is turned off in libstdc++.
@kboyarinov, is my understanding correct (especially for the 3rd bullet)? If so, the PR looks good to me.
I do not have much to add, but I have installed GCC 9/10 and have confirmed that the following program does not compile on the main branch with gcc9/gcc10 and it now successfully compiles with this PR.
#include <cstddef> #include <oneapi/dpl/execution> int main() { return 0; }
One important thing should be noticed about this fix. Me and Ruslan found this known limitation in our documentation during the online review. The formulation is a bit confusing but it seems like it describes the issue I am trying to fix. So this PR should be considered only as a Known Limitation fix. And it can be closed without merging if we decide to keep this known limitation not to complicate our config files.
Also the issue #1061 describes the same problem.
I have the following understanding of the cases of handling incompatibility between TBB backend in libstdc++ v9/v10 and modern TBB, oneTBB, when the later is used:
- oneDPL headers are included first.
PSTL_USE_PARALLEL_POLICIESis set to0; when libstdc++ pulls in its configs, it sets internal macros (__PSTL_USE_PAR_POLICIESand__PSTL_PAR_BACKEND_TBB) according toPSTL_USE_PARALLEL_POLICIESand thus turn offs TBB backend. It disables the parallel policies entirely in libstdc++ v9 (because there is only TBB backend) and fall backs to the serial backend in libstdc++ v10.- oneDPL headers are included after an STL header from libstdc++ (e.g.
execution). It is too late, nothing can be done here.- oneDPL headers are included after a non-STL header from libstdc++ (e.g.
cstddef). We can fix it, what this PR does. At this point, libstdc++ has already set the internal macros to use TBB, but it has not started using it. The PR just redefines these internal macros, so TBB is turned off in libstdc++.@kboyarinov, is my understanding correct (especially for the 3rd bullet)? If so, the PR looks good to me.
Bullet number 1 is absolutely correct.
Could you please describe the difference between bullets 2 and 3 because for me both execution and cstddef are STL headers:)
There is also an important thing to notice about execution header. If it is included on the user side before oneDPL - it still fails because no macros (like PSTL_USE_PARALLEL_POLICIES) defined. And there is no solution on the oneDPL side for this use case.
If any other header from STL (not execution) is included before oneapi/dpl/execution, the issue I am trying to fix takes place and the logic is to modify the internal macros to discard the decision made previously in GCC config files.
Could you please describe the difference between bullets 2 and 3 because for me both execution and cstddef are STL headers:)
Sorry, I confused you by saying "STL" or "non-STL" header. I believe these terms do no exist. It is better to use C++ Standard Library (let it be STD) header. Both execution and cstddef are STD or C++ Standard Library headers. Saying "non-STL" I rather meant "a header which is unlikely to include Parallel STL implementation". cstddef does not include Parallel STL (at least in libstdc++). execution includes Parallel STL in many cases. That's the core difference between 2nd and 3rd bullets.
If any other header from STL (not execution) is included before oneapi/dpl/execution, the issue I am trying to fix takes place...
Will the fix work if the user includes e.g. algorithm or numeric header? Will their inclusion result in an accidental use of the incompatible TBB?
Could you please describe the difference between bullets 2 and 3 because for me both execution and cstddef are STL headers:)
Sorry, I confused you by saying "STL" or "non-STL" header. I believe these terms do no exist. It is better to use C++ Standard Library (let it be STD) header. Both
executionandcstddefare STD or C++ Standard Library headers. Saying "non-STL" I rather meant "a header which is unlikely to include Parallel STL implementation".cstddefdoes not include Parallel STL (at least in libstdc++).executionincludes Parallel STL in many cases. That's the core difference between 2nd and 3rd bullets.If any other header from STL (not execution) is included before oneapi/dpl/execution, the issue I am trying to fix takes place...
Will the fix work if the user includes e.g.
algorithmornumericheader? Will their inclusion result in an accidental use of the incompatible TBB?
As far as I understand, inclusion of <algorithm>, <memory> and <numeric> never results in implicit inclusion of <execution> and hence only <execution> header will include TBB. See my experiments here.
So the only use case that will still fail is include <execution> before any other oneDPL headers. And as I said previously, there is nothing we can do with this.
Committed the simplified version of the patch, proposed by @akukanov. @akukanov @MikeDvorskiy @adamfidel @dmitriy-sobolev, could you please take a look one more time?
This patch might also deserve updating our known limitations.