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

[Daniel Sanders]

================
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 */
----------------
Faraz Shahbazker wrote:
> Faraz Shahbazker wrote:
> > 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.
> After deeper inspection, using defined(mips16) is not suitable, because the library does not always need to be built in mips16 mode. The check is based on:
> 1. if an application built with -mips16 can link with _this_ library. For example, if the library uses micromips or an ABI other than o32/o64, then a mips16 application will fail to link with it. I need to add a check for ABI.
> 2. if the function needs to be defined. For example, all soft-fp wrappers are by definition redundant in soft-float mode as the compiler will never generate calls to those wrappers. Similarly, double-precision wrappers are redundant when building with -msingle-float -- this check needs to be added as well.
> 
> I could get rid of the empty-macros by conditionally #defining MIPS16_COMPAT in mips16_gen.h and wrapping contents of mips16_*.c in #ifdef MIPS16_COMPAT. Will include this and other changes in the next revision.
> After deeper inspection, using defined(mips16) is not suitable, because the library does not always need to be built in mips16 mode.

Do you mean the library has to provide these functions regardless of the options used to compile the library just in case it is linked against a MIPS16 object/function that references them?

> if an application built with -mips16 can link with _this_ library. For example, if the library uses micromips or an
> ABI other than o32/o64, then a mips16 application will fail to link with it.

microMIPS and MIPS16 cannot coexist so you don't need to deal with that case. From the microMIPS spec:
  microMIPS is the preferred replacement for MIPS16e. Therefore these two schemes never co-exist within the
  same processor core
and later in the same document they both use the same ISA mode value:
  The MIPS Release 3 architecture defines an ISA mode for each processor. An ISA mode value of 0 indicates MIPS32
  instruction decoding. In processors implementing microMIPS32, an ISA mode value of 1 selects microMIPS32
  instruction decoding. In processors implementing the MIPS16e ASE, an ISA mode value of 1 selects the decoding of
  instructions as MIPS16e

Regarding the ABI comment: You can't check for mixing objects with different ABI's at compile-time. That needs to be a link-time check so you don't need to deal with that case in your code.

> if the function needs to be defined. For example, all soft-fp wrappers are by definition redundant
> in soft-float mode as the compiler will never generate calls to those wrappers. Similarly,
> double-precision wrappers are redundant when building with -msingle-float -- this check needs
> to be added as well.

You generally don't need a check to cover the cases where they are redundant. If they exist but the objects they are in are not referenced in some way then they won't be included in a link.

http://reviews.llvm.org/D4454