[LLVMdev] [PATCH]: MSVC build enhancements

[José Fonseca]

On Sat, Mar 6, 2010 at 9:49 PM, Óscar Fuentes <ofv at wanadoo.es> wrote:
> José Fonseca <jose.r.fonseca at gmail.com> writes:
>
>> Attached are two patches with MSVC build enchancements.
>>
>> They are quite trivial, but were necessary to correctly link LLVM
>> libraries with Mesa3D on Windows.
>
> [snip]
>
>>    add_llvm_definitions( -D_SCL_SECURE_NO_DEPRECATE )
>> +  add_llvm_definitions( -D_SECURE_SCL=0 )
>
> With this setting the default LLVM build becomes incompatible with
> libraries compiled with _SECURE_SCL=1 (which is the default
> setting). The right thing here is to use an option.

OK.

>> -    add_llvm_definitions("/${LLVM_USE_CRT}")
>> +    # http://www.cmake.org/Wiki/CMake_FAQ#How_can_I_build_my_MSVC_application_with_a_static_runtime.3F
>> +    foreach(flag_var
>> +            CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_RELEASE CMAKE_C_FLAGS_MINSIZEREL CMAKE_C_FLAGS_RELWITHDEBINFO
>> +            CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO)
>> +       if(${flag_var} MATCHES "/MD")
>> +          string(REGEX REPLACE "/MD" "/${LLVM_USE_CRT}" ${flag_var} "${${flag_var}}")
>> +       endif(${flag_var} MATCHES "/MD")
>> +    endforeach(flag_var)
>>      message(STATUS "Using VC++ CRT: ${LLVM_USE_CRT}")
>>    endif (NOT ${LLVM_USE_CRT} STREQUAL "")
>
> Ugh! Well, if this is the only way... However, AFAIK the last flag is
> the one that prevails, and that's the one introduced by
> add_llvm_definitions. What's the problem if you don't replace the switch
> everywhere?

It didn't work. I was still getting libraries with the wrong CRT.

I think it's because these flags don't really override each other. My
understanding is that these options work by defining _DEBUG _MT and
_DLL preprocessing symbols
(http://msdn.microsoft.com/en-us/library/2kzt1wy3.aspx) which are
recognized by the CRT headers and used to select the right functions
propotypes and the right CRT .LIB via #pragmas defaultlib. And appears
that defining multiple flags gets you a mixed behavior, instead of
overriding earlier flags.

> Does the replacement method above works well with debug
> builds? (On debug builds, /MDd is used by default instead of /MD, so
> replacing /MD with /MTd, for instance, would give /MTdd)

I've used that snippet from the CMake in several projects and it
appears to work well. But it might be worth checking it to make sure.

>> --- a/include/llvm/System/DataTypes.h.cmake
>> +++ b/include/llvm/System/DataTypes.h.cmake
>> @@ -100,6 +100,9 @@ typedef u_int64_t uint64_t;
>>  #else
>>  #include <math.h>
>>  #endif
>> +#ifdef HAVE_STDINT_H
>> +#include <stdint.h>
>> +#else /* !HAVE_STDINT_H */
>
> This looks incorrect to me. stdint.h is already included below for
> non-MSVC systems. Maybe you inteded to #include it for MSVC only? Or do
> intend to correct some problem with non-MSVC builds too?

The idea here is indeed to use an external stdint.h in MSVC builds.

The problem is that everybody except Microsoft recognized the
usefulness of stdint.h types [1], and they work around it for MSVC by
defining the stdint.h types themselves. LLVM does it. Mesa3D did it
too. But that won't work when two such projects try to interoperate --
you'll get errors that uint8_t/uint16_t/uint32_t/etc has been
redefined.

There is more than one way to address this. The solution I came up
with was to make all projects respect HAVE_STDINT_H, and only define
the stdint types if HAVE_STDINT_H is not defined. Then all projects
can use the same definition, without symbol clashes. We actually ship
a tiny stdint.h in Mesa3D,
http://cgit.freedesktop.org/mesa/mesa/tree/include/c99/stdint.h , but
there are many free replacements out there.

This is not a problem specific to Mesa and LLVM -- it is a problem
faced by any pair of projects that use stdint types and want to build
on MSVC.

Jose

[1] I'm being unfair. Microsoft certainly recognized by now how useful
the stdint.h types are to write portable code, which is probably the
reason why they haven't been implemented yet. It doesn't appear C99
will be implemented soon either:
https://connect.microsoft.com/VisualStudio/feedback/details/485416/support-c99