[PATCH] D13384: Get compiler-rt/builtins to build on MSVC

Saleem Abdulrasool via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 2 20:37:49 PDT 2015


compnerd added a subscriber: compnerd.
compnerd added a comment.

This really feels like a number of different changes put into a single one.  Please split this up.


================
Comment at: lib/builtins/enable_execute_stack.c:26
@@ +25,3 @@
+#include <wtypes.h>
+#endif
+
----------------
If wtypes.h is needed/used, just include it.  Don't wrap it in the check for already included.

================
Comment at: lib/builtins/enable_execute_stack.c:30
@@ -24,3 +29,3 @@
 #include <windef.h>
-#include <winbase.h>
+#endif
 #else
----------------
Similar.

================
Comment at: lib/builtins/int_math.h:75
@@ -46,1 +74,3 @@
+#define crt_isinf(x) __builtin_isinf((x))
+#define crt_isnan(x) __builtin_isnan((x))
 
----------------
This would be easier to follow if they were grouped rather than a single huge MSVC vs not block.

================
Comment at: lib/builtins/int_types.h:168
@@ -143,1 +167,3 @@
+#define _Cimag(x) (x).im
+#endif
 #endif /* INT_TYPES_H */
----------------
clang-format the change

_[A-Z] is language reserved, please change the naming on these.

================
Comment at: lib/builtins/int_util.h:30
@@ +29,3 @@
+#define __UNUSED_GNU __attribute__((unused))
+#endif
+
----------------
This really complicates the usage.  Im wondering if it may be useful to write this as a wapping macro instead.


http://reviews.llvm.org/D13384





More information about the llvm-commits mailing list