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

Issue with VS 2017 15.8: _ENABLE_EXTENDED_ALIGNED_STORAGE vs _DISABLE_EXTENDED_ALIGNED_STORAGE #249

Open
shlublu opened this issue Aug 16, 2018 · 2 comments

Comments

@shlublu
Copy link

shlublu commented Aug 16, 2018

The latest update of Microsoft Visual Studio 2017 (VS 2017 15.8, 14th of Aug 2018) makes the following error message to appear when I compile any project using Shark 4.0.0:

You've instantiated std::aligned_storage<Len, Align> with an extended alignment (in other
words, Align > alignof(max_align_t)). Before VS 2017 15.8, the member type would
non-conformingly have an alignment of only alignof(max_align_t). VS 2017 15.8 was fixed to
handle this correctly, but the fix inherently changes layout and breaks binary compatibility
(only for uses of aligned_storage with extended alignments).
Please define either
(1) _ENABLE_EXTENDED_ALIGNED_STORAGE to acknowledge that you understand this message and
that you actually want a type with an extended alignment, or
(2) _DISABLE_EXTENDED_ALIGNED_STORAGE to silence this message and get the old non-conformant
behavior.

This is a static_assert(_Always_false<_Aligned>). Looks like this comes from <shark_includes>/linalg/blas/kernels/default/mgemm.hpp but I'm not 100% sure whether this is the only file that is concerned. Should this be needed I kept the full log of what happened, I can send it to you.

My local Shark 4.0.0 was installed according to the official guidelines available at http://www.shark-ml.org/sphinx_pages/build/html/rest_sources/installation.html#windows-and-visual-studio
but with one difference, not relevant here I think: I'm using Boost 1.67 instead of Boost 1.59 mentioned in the document above.

As I need to move forward on my project while Shark 4.0.0 works perfectly in my environment so far, I'm currently using _DISABLE_EXTENDED_ALIGNED_STORAGE for now in my project. This seems to work properly. I'll try to recompile both Shark 4.0.0 and my project with _ENABLE_EXTENDED_ALIGNED_STORAGE later when I'll have more time.

However it would be interesting to:

  • provide a few guidelines regarding this (very new) issue in Shark's installation documentation.
  • determine which behaviour is suitable between ENABLE/DISABLE_EXTENDED_ALIGNED_STORAGE to have it defined straight in Shark's codebase.

Thanks a lot for everything anyway. You guys do a great job and this lib is awesome!

@Ulfgard
Copy link
Member

Ulfgard commented Aug 17, 2018

Hi,

Thanks for reporting this!

I think the correct behaviour should be given by _ENABLE_EXTENDED_ALIGNED_STORAGE. We wanted to have 16 byte alignment in GEMM so that the compiler could use fast SSE/AVX intrinsics. The old behaviour/disable flag will prevent the compiler from doing this.

I have no windows machine to test until Sunday. We will issue a bugfix release 4.0.1 where this is handled properly.

@shlublu
Copy link
Author

shlublu commented Aug 17, 2018

Sounds great, thank you!
At that stage, I can just confirm that _DISABLE_EXTENDED_ALIGNED_STORAGE works, which is not surprising as this is the way it used to be (but silently) before. However this looks a bit patchy-dirty :)

I won't have a chance to recompile my whole stuff (local Shark + client projects) using
_ENABLE_EXTENDED_ALIGNED_STORAGE before a few weeks actually. But I'll give a try and let you know should I come back before the 4.0.1 !

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

No branches or pull requests

2 participants