[PATCH] D25536: [APFloat] Make APFloat an interface class to the internal IEEEFloat. NFC.

[Eric Christopher via llvm-commits]

echristo added a comment.

In https://reviews.llvm.org/D25536#568978, @hfinkel wrote:

> In https://reviews.llvm.org/D25536#568792, @timshen wrote:
>
> > In https://reviews.llvm.org/D25536#568766, @hfinkel wrote:
> >
> > > To be clear, you plan is to later change:
> > >
> > >   union {
> > >     const fltSemantics *semantics;
> > >     IEEEFloat IEEE;
> > >   };
> > >   
> > >
> > > to be something like:
> > >
> > >   union {
> > >     const fltSemantics *semantics;
> > >     IEEEFloat IEEE;
> > >     PPCDoubleDouble PPCDD;
> > >   };
> > >   
> > >
> > > and then make:
> > >
> > >   opStatus divide(const APFloat &RHS, roundingMode RM) {
> > >     return IEEE.divide(RHS.IEEE, RM);
> > >   }
> > >   
> > >
> > > into:
> > >
> > >   opStatus divide(const APFloat &RHS, roundingMode RM) {
> > >     return isPPCDoubleDouble() ? PPCDD.divide(RHS, RM) : IEEE.divide(RHS, RM);
> > >   }
> > >   
> > >
> > > or similar?
> >
> >
> > Yes, this is the plan.
>
>
> Makes sense to me. Opinions from anyone else?


Seems reasonable and the easiest solution here.

-eric

In https://reviews.llvm.org/D25536#568978, @hfinkel wrote:

> In https://reviews.llvm.org/D25536#568792, @timshen wrote:
>
> > In https://reviews.llvm.org/D25536#568766, @hfinkel wrote:
> >
> > > To be clear, you plan is to later change:
> > >
> > >   union {
> > >     const fltSemantics *semantics;
> > >     IEEEFloat IEEE;
> > >   };
> > >   
> > >
> > > to be something like:
> > >
> > >   union {
> > >     const fltSemantics *semantics;
> > >     IEEEFloat IEEE;
> > >     PPCDoubleDouble PPCDD;
> > >   };
> > >   
> > >
> > > and then make:
> > >
> > >   opStatus divide(const APFloat &RHS, roundingMode RM) {
> > >     return IEEE.divide(RHS.IEEE, RM);
> > >   }
> > >   
> > >
> > > into:
> > >
> > >   opStatus divide(const APFloat &RHS, roundingMode RM) {
> > >     return isPPCDoubleDouble() ? PPCDD.divide(RHS, RM) : IEEE.divide(RHS, RM);
> > >   }
> > >   
> > >
> > > or similar?
> >
> >
> > Yes, this is the plan.
>
>
> Makes sense to me. Opinions from anyone else?





https://reviews.llvm.org/D25536