[PATCH] D26183: [compiler-rt] builtins: Allow building windows arm functions for mingw

Saleem Abdulrasool via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 18 12:41:31 PST 2016


compnerd added inline comments.


================
Comment at: lib/builtins/arm/aeabi_idivmod.S:20
+#define __aeabi_idivmod __rt_sdiv
+#endif
+
----------------
mstorsjo wrote:
> compnerd wrote:
> > Id rather sink this around the `DEFINE_COMPILERRT_FUNCTION`.  There is a single use, and that makes it more apparent that we are trying to rename the function.
> Well, the same name is used at the end in `END_COMPILERRT_FUNCTION` as well, although that's a no-op on non-elf platforms. Do you still want the ifdef moved around `DEFINE_COMPILERRT_FUNCTION`, even though it no longer properly matches the corresponding no-op `END_COMPILERRT_FUNCTION` in that case?
Thats a good point.  Lets avoid that then.


================
Comment at: lib/builtins/arm/aeabi_idivmod.S:18
 
+#ifdef __MINGW32__
+#define __aeabi_idivmod __rt_sdiv
----------------
Can you please use the

    #if defined(__MINGW32__)

form?  It makes modifying it easier in the future.  (Throughout)


================
Comment at: lib/builtins/mingw_fixfloat.c:68
+    return __floatundisf(a);
+}
----------------
Please clang-format this file.


https://reviews.llvm.org/D26183





More information about the llvm-commits mailing list