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

Faraz Shahbazker Faraz.Shahbazker at imgtec.com
Mon Jul 14 01:59:00 PDT 2014


Mostly agreed, further inputs on first point?

================
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);
+
----------------
Daniel Sanders wrote:
> 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.
I've used only [[ https://gcc.gnu.org/onlinedocs/gccint/Soft-float-library-routines.html#Soft-float-library-routines | this document ]] as my reference. 
Excerpt: ``Do not rely on this implementation; only the semantics documented below are guaranteed. ``

It seems like glibc's strategy is what is best-suited for a pure soft-float, whereas this implementation relies directly on the h/w comparison operations. As an example, this version doesn't need to check ordered-ness for every comparison operation because the underlying h/w operation takes care of it.

Do you foresee any disadvantages with this approach? I could compare space & time for both implementations, just to make sure we're not losing out.

================
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.
+ *
----------------
Daniel Sanders wrote:
> 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.
How about this (appropriately modified for all source files):

```
This file contains generator macros for soft-fp wrappers to make hardware 
floating point available to MIPS16 code by temporarily switching the instruction set.
These wrappers accept arguments and produce results in GP registers, but may 
perform the underlying operation using floating point instructions and registers.
```

================
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 */
----------------
Daniel Sanders wrote:
> 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.
Agreed on both counts.

http://reviews.llvm.org/D4454






More information about the llvm-commits mailing list