[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:
> Daniel Sanders wrote:
> > 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.
> >>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?
> Yes. These functions should be present by default, excluding specific cases where we know they are not needed or will not work. Excluded cases would be more obvious if the implementation were in assembly (as it is for libgcc) because they would either fail to build or behave incorrectly.
> 
> I think there is no harm in these functions being always available with every thing resolved correctly at link time. However, if exclusion checks are cheap and easy, we prefer to use them wherever possible simply to cut down on time needed for building/testing those cases. Even seemingly trivial savings in this regard can be significant on target boards.
Sorry for going quiet all of a sudden. Things got rather busy.

That makes sense to me. Excluding these functions when building the library for microMIPS or MIPS32r6/MIPS64r6 seems sensible too. I'm not sure about the soft-float check though. Admittedly it would be unusual, but I don't think there's a technical reason that an MIPS16 app couldn't be compiled with -mhard-float and link against a -msoft-float library. I believe it should end up with the app calling the hardfloat wrappers which are implemented using softfloat.

http://reviews.llvm.org/D4454