[PATCH] [compiler-rt][builtins][MIPS] Add mips16 wrappers for float arithmetic, comparison and conversion

Daniel Sanders daniel.sanders at imgtec.com
Fri Jul 11 02:52:58 PDT 2014


================
Comment at: lib/builtins/mips/mips16_cmpdf2.c:26-32
@@ +25,9 @@
+
+GENOP2(gtdf2, >, 1, 0);
+
+GENOP2(gedf2, >=, 0, -1);
+
+GENOP2(ltdf2, <, -1, 0);
+
+GENOP2(ledf2, <=, 0, 1);
+
----------------
Shouldn't gtdf2 and gedf2 be the same function and return -2, -1, 0, or 1 for unordered, less-than, equal, or greater-than respectively (like glibc's soft float does)?

Similarly for ltdf2/ledf2 and the single precision equivalents.

================
Comment at: lib/builtins/mips/mips16_gen.h:12-14
@@ +11,5 @@
+ *
+ * This file contains macros to generate soft-fp wrappers for mips16. These 
+ * wrappers accept arguments in GP registers and invoke underlying operation
+ * using FP instructions and registers.
+ *
----------------
I understand what you mean but I think this ought to explain that the soft-fp wrappers make hardware floating point available to MIPS16 despite MIPS16 not supporting hardware floating point by temporarily switching the instruction set.

The header comments in the other files should mention this too.

================
Comment at: lib/builtins/mips/mips16_gen.h:22-23
@@ +21,4 @@
+
+#if defined(__mips_micromips) || defined(__mips_soft_float) \
+    || __mips_isa_rev >= 6
+  /* DO NOTHING */
----------------
This doesn't look right to me. Why not test using '#if !defined(__mips16) || defined(__mips_soft_float)'?

Also, I think it's easier to understand if the content of mips16_*.c is wrapped in an '#if defined(__mips16)' rather than defining a macro to expand to nothing.

http://reviews.llvm.org/D4454






More information about the llvm-commits mailing list