biotrump / libyuv

Automatically exported from code.google.com/p/libyuv
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

clang-cl /fallback can produce link errors #469

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. introduce a compile error in a source file
2. compile with clang for windows

What is the expected output?
libyuv should still work, if the visual c build succeeds.

What do you see instead?
link errors due to functional differences, depending on source file.

Please use labels and text to provide additional information.
header files are primary concern.
e.g. row.h contains Visual C specific functions.

Original issue reported on code.google.com by fbarch...@chromium.org on 22 Jul 2015 at 11:01

GoogleCodeExporter commented 9 years ago
Rotate now uses Visual C source for 32 bit clangcl.
https://webrtc-codereview.appspot.com/54819004/

Was

// The following are available for Visual C:
#if !defined(LIBYUV_DISABLE_X86) && defined(_M_IX86) && \     
    defined(_MSC_VER) && !defined(__clang__)

Now

// The following are available for Visual C and clangcl 32 bit:
#if !defined(LIBYUV_DISABLE_X86) && defined(_M_IX86)

For 32 bit, this would make Visual C and clangcl use the same code path, 
avoiding this issue.

Consider using Visual C source for all clangcl 32 bit:
compare_win.cc
rotate_win.cc
row_win.cc
scale_win.cc

Original comment by fbarch...@chromium.org on 11 Aug 2015 at 5:17

GoogleCodeExporter commented 9 years ago
Switched clang to use Visual C for
compare_win.cc
rotate_win.cc

Todo:
row_win.cc
scale_win.cc

Once complete, /fallback will work for 32 bit.

Original comment by fbarch...@chromium.org on 18 Aug 2015 at 6:26

GoogleCodeExporter commented 9 years ago
scale fixed.

To repro, change build/common.gypi to add /fallback

            ['clang==1 and target_arch=="ia32"', {
              # TODO(thakis): Remove this block once llvm.org/PR24167 is fixed.
              'VCCLCompilerTool': {
                'WarnAsError': 'false',
                'AdditionalOptions': [
                  '/fallback',
                ],
              },
            }],

and change calling code - eg. scale.cc to include an error:

#if defined(__clang__) && defined(_MSC_VER)
#error fallback
#endif

The low levels will use clang but the high level will use visual c.
The mismatch will cause link errors if there are differences.

Original comment by fbarch...@chromium.org on 18 Aug 2015 at 9:55

GoogleCodeExporter commented 9 years ago
fixed for 32 bit in r1470

64 bit uses C for visual C and asm for clangcl, so no immediate plans to make 
them the same.

Original comment by fbarch...@chromium.org on 19 Aug 2015 at 6:55