__declspec(align()) is illegal on functions, fatal error on VS 2015

[GoogleCodeExporter]

VS 2015 gives hundreds of errors about illegal use of __declspec(align) on 
function declarations. It appears that this was never supported:
https://msdn.microsoft.com/en-us/library/83ythb65.aspx
and it is now a fatal error. The error messages look like this:

src\third_party\libyuv\source\row_win.cc(4640): error C3323: 'alignas'
and '__declspec(align)' are not allowed on function declarations

This is blocking the porting of Chrome to VS 2015.

The errors show up in row_win.cc, scale_win.cc, compare_win.cc, and
possibly other source files.

Original issue reported on code.google.com by brucedaw...@chromium.org on 8 Apr 2015 at 12:48

[GoogleCodeExporter]

Testing with VS 2013 shows that functions are normally/often aligned to 16 
bytes, regardless of whether they are preceded by __declspec(align(16)). Adding 
__declspec(align(32)) also makes no difference.

Original comment by brucedaw...@chromium.org on 8 Apr 2015 at 12:52

[GoogleCodeExporter]

Theres a 1 cycle penalty if a branch target instruction spans a cache line on 
older CPUs and compilers.
But not a big deal any more, so we can probably go ahead and remove it.

Prefer a way to test it.  Guess I'll check it in and have someone verify later.
It will require a chromium deps roll.

Original comment by fbarch...@chromium.org on 8 Apr 2015 at 2:47

• Changed state: Accepted

[GoogleCodeExporter]

From VS 2010 to VS 2015 I see no evidence that __declspec(align()) makes any 
difference to code alignment. I see 16-byte alignment whether the requested 
alignment is 0, 16, or 32. If __declspec(align()) does make a difference then 
macro below can be used. But if __declspec(align()) makes no difference then it 
should be deleted and there will be no penalty.

#if _MSC_VER < 1900
#define ALIGNCODE __declspec(align(32))
#else
#define ALIGNCODE
#endif

Original comment by brucedaw...@google.com on 8 Apr 2015 at 4:49

[GoogleCodeExporter]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1122900 where I ran into this 
issue when building Gecko with VS2015, and where I wrote the (trivial) patch to 
do it. I've attached the same patch here so that it is subject to my 
contributor agreement. Note that the patch is a patch against Firefox, but you 
should be able to apply it to Chromium with the appropriate "patch -p" 
invocation.

Original comment by brian@briansmith.org on 8 Apr 2015 at 10:46

Attachments:

• remove-decltype-libyuv.patch

[GoogleCodeExporter]

Note: Brian's patch against Mozilla is against our last import of libyuv, which 
is rev 971 (though likely it will apply with fuzzing/offsets)

Original comment by randell....@gmail.com on 9 Apr 2015 at 5:43

[GoogleCodeExporter]

r1365 removes all code declspec(align(16))

Original comment by fbarch...@chromium.org on 13 Apr 2015 at 12:01

• Changed state: Fixed

[GoogleCodeExporter]

Almost...

The changes have rolled into Chrome but row_win.cc was missed. It contains 
three __declspec(align(32)) statements.

I'd mark the bug as unfixed but I don't seem to have permissions.

Original comment by brucedaw...@chromium.org on 16 Apr 2015 at 8:22

[GoogleCodeExporter]

right.  changed to unfixed.  fix coming.

Original comment by fbarch...@chromium.org on 20 Apr 2015 at 5:41

• Changed state: Started

[GoogleCodeExporter]

r1374 fixes libyuv end of it.
This is the roll CL for chromium
https://codereview.chromium.org/1094223002/

Original comment by fbarch...@google.com on 21 Apr 2015 at 5:43