YuvConstants declare alignment for externs as well as declarations

YuvConstants declare alignment for externs as well as declarations

On visual c 2013 and earlier a warning is generated if externs
are not declared with the same alignment as the declaration, when
using /ltcg

BUG=libyuv:633
TEST=standalong test built with cl /Bv /GL /Ox /nologo a.cc b.cc /link /ltcg
R=skal@google.com

Committed: https://chromium.googlesource.com/libyuv/libyuv/+/de944ed8c74909ea6fbd743a22efe1e55e851b83

[fbarchard1, 2016-08-30 00:42:28]

chrome builds only support vs2015 so tested this externally

a standalone test:
a.h
extern const int zippy;

a.cc:
#include "a.h"
const int __declspec(align(32)) zippy = 3;

b.cc
#include "a.h"

#include <stdio.h>

int main() {
  printf("%d\n", zippy);
}

cl /Bv /GL /Ox /nologo a.cc b.cc /link /ltcg

warning C4742: 'int const zippy' has different alignment in
'c:\work\test4744\b.cc' and 'c:\work\test4744\a.cc': 4 and 32
warning C4744: 'int const zippy' has different type in 'c:\work\test4744\b.cc'
and 'c:\work\test4744\a.cc': 'int' and '__declspec(align(32)) int'


adding __declspec(align(32)) to the extern in the .h fixes the issue.

[skal, 2016-08-30 05:03:14]

https://codereview.chromium.org/2291533004/diff/1/include/libyuv/row.h
File include/libyuv/row.h (right):

https://codereview.chromium.org/2291533004/diff/1/include/libyuv/row.h#newcod...
include/libyuv/row.h:465: extern const struct YuvConstants
SIMD_ALIGNED(kYuvI601Constants);  // BT.601
why is there two difference macros (SIMD_ALIGNED / SIMD_ALIGNED32) ? Why not
just one?

Because, looking at the code, it seems that:
  * SIMD_ALIGNED32 is never used for aarch64 / arm, and
  * SIMD_ALIGNED is never used for x86

With just one macro, you wouldn't need these ifdefs here.

[fbarchard1, 2016-08-30 17:41:23]

On 2016/08/30 05:03:14, skal wrote:
> https://codereview.chromium.org/2291533004/diff/1/include/libyuv/row.h
> File include/libyuv/row.h (right):
> 
>
https://codereview.chromium.org/2291533004/diff/1/include/libyuv/row.h#newcod...
> include/libyuv/row.h:465: extern const struct YuvConstants
> SIMD_ALIGNED(kYuvI601Constants);  // BT.601
> why is there two difference macros (SIMD_ALIGNED / SIMD_ALIGNED32) ? Why not
> just one?
> 
> Because, looking at the code, it seems that:
>   * SIMD_ALIGNED32 is never used for aarch64 / arm, and
>   * SIMD_ALIGNED is never used for x86
> 
> With just one macro, you wouldn't need these ifdefs here.

AVX2 requires alignment of 32 for some operations.  But 16 is enough for SSE2,
NEON.  Older compilers didnt support 32 byte alignment.  And the related
attribute vector_size(32).  So I've tried to limit vector and alignment of 32 to
code that uses AVX2, which is ifdef'ed.

Looking at how I've used SIMD_ALIGNED, I could use a macro that varies in size. 
But the definition is based on compiler - declspec vs attribute.  And the size
is based on the SIMD instruction set.  So I the definition of align would need
to be done carefully.  Or just always 32 and not worry about the wasted
perf/memory.

So conclusion, I agree it would make the code using SIMD_ALIGN easier, but will
defer with a TODO for now.  I've added the todo.

[fbarchard1, 2016-08-30 17:52:59]

row.h defines SIMD_ALIGNED to 32 byte alignment for compilers supporting AVX2. 
Else 16.

https://codereview.chromium.org/2291533004/diff/1/include/libyuv/row.h
File include/libyuv/row.h (right):

https://codereview.chromium.org/2291533004/diff/1/include/libyuv/row.h#newcod...
include/libyuv/row.h:465: extern const struct YuvConstants
SIMD_ALIGNED(kYuvI601Constants);  // BT.601
On 2016/08/30 05:03:14, skal wrote:
> why is there two difference macros (SIMD_ALIGNED / SIMD_ALIGNED32) ? Why not
> just one?
> 
> Because, looking at the code, it seems that:
>   * SIMD_ALIGNED32 is never used for aarch64 / arm, and
>   * SIMD_ALIGNED is never used for x86
> 
> With just one macro, you wouldn't need these ifdefs here.

Done.

[skal, 2016-08-30 18:05:55]

LGTM

great, simplified!

[fbarchard1, 2016-08-30 18:06:49]

Description was changed from

==========
YuvConstants declare alignment for externs as well as declarations

On visual c 2013 and earlier a warning is generated if externs
are not declared with the same alignment as the declaration, when
using /ltcg

BUG=libyuv:633
TEST=standalong test built with cl /Bv /GL /Ox /nologo a.cc b.cc /link /ltcg
R=skal@google.com
==========

to

==========
YuvConstants declare alignment for externs as well as declarations

On visual c 2013 and earlier a warning is generated if externs
are not declared with the same alignment as the declaration, when
using /ltcg

BUG=libyuv:633
TEST=standalong test built with cl /Bv /GL /Ox /nologo a.cc b.cc /link /ltcg
R=skal@google.com

Committed:
https://chromium.googlesource.com/libyuv/libyuv/+/de944ed8c74909ea6fbd743a22e...
==========

[fbarchard1, 2016-08-30 18:06:50]

Committed patchset #3 (id:40001) manually as
de944ed8c74909ea6fbd743a22efe1e55e851b83 (presubmit successful).