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

port to Windows & MSVC #119

Closed
mattkretz opened this issue Apr 8, 2016 · 1 comment
Closed

port to Windows & MSVC #119

mattkretz opened this issue Apr 8, 2016 · 1 comment

Comments

@mattkretz
Copy link
Member

mattkretz commented Apr 8, 2016

Simple request: Vc and all its unit tests and examples should build and pass with the latest MSVC compiler.
Complex task: MSVC has a lot of quirks in it's implementation/interpretation of the C++ standard. Therefore, lot's of C++(11) code in Vc that builds fine on all other systems fails with incomprehensible error messages when MSVC compiles it.

This issue will be used to collect

  1. the shortcomings of MSVC
  2. possible workaround
  3. necessary trade-offs
  4. bug reports submitted to Microsoft

Issue table

Issue Workaround Bug Report
In some cases MSVC will see a hard substitution failure when Vc::enable_if<X> is used instead of doing SFINAE. Substituting Vc::enable_if<X> with typename std::enable_if<X>::type makes SFINAE work again.
Explicit template instantiation of operators of template types normally requires the template keyword before the operator. MSVC rejects the template keyword. Remove the template keyword. This incorrect code does what is intended. 2591761
Use of class member types in member function arguments outside of the class declaration fail the build. Replace the short type with the long type expression it aliases.
Use of unqualified type names is sometimes considered ambiguous if the type name exists in at least two namespaces. Fully qualify the namespace.
Use of constexpr functions in template arguments is considered not a constant expression. Use a constexpr variable. 2390989
A mismatch of the template parameter names in the forward declaration and declaration of a class template can lead to compilation errors. Use the same names for the template parameters.
Subtle differences in overload resolution rules. I cannot say I have understood the MSVC rules. #ifdef some overloads by guessing until it compiles
Long function names are truncated. Long function names can be generated from huge Typelist template arguments. Pray and hope that no name clashes will occur.
Function parameter types may not use alignas, or other means of over-alignment. Use the __vectorcall calling convention
A deleted function used in template deduction can lead to a hard failure instead of SFINAE. 2229371 (Fixed in 2015 Update 2)
Compilation errors point to unrelated code. Curse and do random edits until it compiles.
MMX intrinsics don't compile even though <mmintrin.h> is included. Rewrite the code without MMX.
sizeof(expression_that_fails_substitution<T>) is evaluated to be 0 and the enclosing expression evaluated. If the this happens to be the denominator of std::ratio, then a static_assert is triggered. Instead the function should have been removed from the overload set via SFINAE. Add a wrapper around the construction of std::ratio to ensure its denominator argument is never 0.
Using a typedef alias for a class template function definition outside of the class declaration does not compile Replace the short alias name with the fully qualified class template with according template arguments.
The empty parameter pack case in recursive implementations of variadic template functions complains about a missing function definition. Add incorrect overloads for the case of empty packs.
Member function implementations inside the class template declaration cannot call member functions of the same class template but with different template arguments. In some cases it helps to move the function definition after the class declaration. In other cases I found no workaround and had to rewrite the code with an inefficient store - load algorithm instead.
mattkretz added a commit that referenced this issue Apr 13, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 13, 2016
Clang does not find the `operator bool` function. An explicit
static_cast to `bool` resolves that issue.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 13, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 13, 2016
Apparently the function names generated from instantiating with long
Typelists are too long for Microsofts taste. There's not much I can/want
to do about it, so just ignore the warning.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 15, 2016
- MSVC rejects the template keyword on operator template instantiation.
  See also:
  https://connect.microsoft.com/VisualStudio/feedback/details/2591761
- MSVC considered the overload set of the test and test3 functions to be
  ambiguous. There's a nice workaround that uses `std::is_const<A>` as
  return type and can thus drop all the const parameter overloads. This
  also works on GCC 5 and clang 3.8.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 15, 2016
The decltype expression on operator-> and operator* makes MSVC crash.
This workaround declares two member types that should work correctly as
return types for operator-> and operator*.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 15, 2016
MSVC does overload resolution differently and takes the const U *mem
overload (I hope) when the argument is U[N].

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 15, 2016
allocator_traits<std::allocator<Vc::SSE::Vector<T>>> fails to compile in
the code generated from the test code using std::deque. Since I have no
idea for a workaround, this commit simply disables the std::deque test
on MSVC.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 15, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 15, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
This fixes Intel CPU model detection on Windows.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
This should be solved in the AVX/SSE implementations now, since Intel
and GCC also started to declare all intrinsics regardless of -m flags

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
... even though the C++ standard says 'struct array'. Ah well, the only
compiler that really cares about the difference gets it wrong.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
The Visual C++ compiler doesn't accept constexpr functions in template
arguments. Size as constexpr data member appears to work fine, though.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Apparently global use of __vectorcall with MSVC is the better
workaround.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
If the default ctor is defaulted MSVC complains about it being
defined twice. Taking it out for MSVC leads to the code I wanted.
Gosh...

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Apr 22, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
MSVC warns about an immediate value that would be used here if the else
branch could ever be reached with this value - which it cannot.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
MSVC makes the ::type access in the alias template a hard error instead
of SFINAE. Moving the ::type access into the places where the alias
template is used resolves MSVC's confusion.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
In some cases MSVC will see a hard substitution failure when
Vc::enable_if<X> is used instead of doing SFINAE. Substituting
Vc::enable_if<X> with typename std::enable_if<X>::type makes SFINAE work
again.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Whenever the ratio in the return type of the size_t operator[] is
encountered with a sizeof expression that fails, MSVC decides to
substitute a 0 for the sizeof instead of just leaving the ratio
instantiation alone via proper SFINAE. The make_ratio helper ensures
that the 0 from the sizeof failure does not reach the denominator of
std::ratio where it would hit a static_assert.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
MSVC doesn't find the 2-argument shifted member function from inside the
rotated and reversed implementations. Moving the function definition
after the class template declaration only helps for some SimdArray
instantiations. The only fix that fully works is to avoid calling
shifted altogether. Therefore, this now uses store-load with MSVC.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
* No inline asm on MSVC. This will likely not work as efficient.
* Replace always_inline attribute with Vc_ALWAYS_INLINE.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
std::array iterators don't have to implicitly convert to bool*.
Apparently the lib(std)c++ implementations use a bool* as iterator.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
This requires a conversion operator to long long for 64-bit argument
usage.

Also `x = x` was not good enough for MSVC as an alternative to `asm(""
::"m"(x))`.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 6, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 7, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 7, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 7, 2016
Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 7, 2016
MSVC would take the wrong function template overload and then error on
template instantiation. I didn't find a workaround to have it pick the
right overload other than refactoring the mechanism to not use enable_if
on L but rather to encode L in a tag argument.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 10, 2016
The tuple_size initializer doesn't have a this pointer, so EDG seems to
be right about complaining that std::tie can't work here.
std::make_tuple OTOH still works, so let's go with that instead. All we
need to know is the number of elements, anyway.

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
mattkretz added a commit that referenced this issue Oct 11, 2016
1. Make sure, in a mingw env, to replace / with - for compiler flags
2. Make the use of `env LANG=C` optional, depending on the
   availability of env
3. Drop the -o flag for MSVC (the compiler complains, that it's
   deprecated)
4. Add regex matches for errors C2676, C2677, and C2679 to recognize
   expected failure

Refs: gh-119
Signed-off-by: Matthias Kretz <kretz@kde.org>
@mattkretz mattkretz added this to the Vc 1.3 milestone Oct 11, 2016
@mattkretz
Copy link
Member Author

For the most part, this issue is resolved. There are a few remaining failing tests and 32-bit Windows doesn't compile at all. Those issues should get their own github issue, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant