Skip to content
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

Use int = 0 SFINAE in <memory> to improve compiler throughput #2124

Merged
merged 14 commits into from
Aug 22, 2022

Conversation

fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Aug 15, 2021

Fixes #248

Similar to https://github.com/microsoft/STL/pull/244/files

and transition now applies only to CUDA:

STL/stl/inc/functional

Lines 847 to 851 in a9321cf

#ifdef __CUDACC__ // TRANSITION, CUDA
#define _USE_FUNCTION_INT_0_SFINAE 0
#else
#define _USE_FUNCTION_INT_0_SFINAE 1
#endif // __CUDACC__

@fsb4000 fsb4000 requested a review from a team as a code owner August 15, 2021 06:50
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Aug 16, 2021
@StephanTLavavej StephanTLavavej self-assigned this Aug 18, 2021
stl/inc/functional Outdated Show resolved Hide resolved
@fsb4000
Copy link
Contributor Author

fsb4000 commented Jun 18, 2022

I tested the constructor:

template <class _Fx, class _Alloc, typename _Mybase::template _Enable_if_callable_t<_Fx, function> = 0>
function(_Fx&& _Func) {
    this->_Reset(_STD forward<_Fx>(_Func));
}
#include <cstdio>
#include <functional>
using namespace std;

void test() {
    puts("hi");
}

int main() {
    function<void()> f = test;
    f();
}
$ nvcc -I"..\out\build\x64\out\inc" main.cpp
main.cpp
$ a.exe
hi

If someone knows what was the original repro it would be nice to test it.

@StephanTLavavej
Copy link
Member

Regarding CUDA, I believe we retained the workaround out of caution, not because we had an actual repro. Now that we've updated to CUDA 11.6, removing the workaround seems like the right thing to do (we can always put it back if necessary).

@StephanTLavavej StephanTLavavej removed their assignment Aug 17, 2022
@StephanTLavavej StephanTLavavej changed the title Use "int = 0" SFINAE in <memory> to improve compiler throughput Use int = 0 SFINAE in <memory> to improve compiler throughput Aug 17, 2022
@StephanTLavavej StephanTLavavej self-assigned this Aug 18, 2022
@StephanTLavavej

This comment was marked as outdated.

@StephanTLavavej StephanTLavavej removed their assignment Aug 18, 2022
@StephanTLavavej

This comment was marked as resolved.

@StephanTLavavej
Copy link
Member

Reported VSO-1595465 "/clr /std:c++17 ICE with int = 0 SFINAE", pushed a conflict-free merge with main, then pushed a workaround to switch between the old code and new code depending on _M_CEE. (The type change from void to int is unconditional, that part is fine for /clr.)

@StephanTLavavej StephanTLavavej self-assigned this Aug 20, 2022
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 22d6408 into microsoft:main Aug 22, 2022
@StephanTLavavej
Copy link
Member

Thanks for modernizing these last remaining traces of old-style SFINAE! 🚀 ✨ 😺

@fsb4000 fsb4000 deleted the fix248 branch August 23, 2022 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<memory>: Consistently use int = 0 SFINAE
3 participants