[PPC, APFloat] Add full PPCDoubleDouble to APFloat

I have found some internal test failures due to the wrong constant folding on ppc_fp128.

As documented in APFloat::PPCDoubleDouble, APFloat doesn’t support PowerPC double-double correctly <https://github.com/llvm-mirror/llvm/blob/492acdd450bcdf9837494d6da029ed064f14fc33/lib/Support/APFloat.cpp#L74>.

To support this, we need to add a second tuple of (sign, exponent, significand) to APFloat. I wonder where should I start to change, so that it’s less hacky? I certainly expect refactoring to come. :slight_smile:

Thanks!

Hi Tim,

How, in general, are you thinking about doing this? I ask because, as you clearly know, the double-double format is formed by the sum of two double-precision numbers, and the various arithmetic operations are formed mostly in terms of double-precision arithmetic on the elements of the pairs. As a result, I suspect we really just want to store the two double-precision numbers, and essentially delegate to APFloat IEEEdouble in the implementation of the various operations.

As such, one option might be that, when PPCDoubleDouble fltSemantics are selected, the exponent and sign are ignored, and we just store the data for the two double-precision floating-point numbers in the significand. We might even do this directly, by making the significand something like this:

  union Significand {
    integerPart part;
    integerPart *parts;
    APFloat *fltparts; // used for PPCDoubleDouble
  } significand;

Thanks again,
Hal

Hi Hal,

Hi Tim,

How, in general, are you thinking about doing this? I ask because, as you clearly know, the double-double format is formed by the sum of two double-precision numbers, and the various arithmetic operations are formed mostly in terms of double-precision arithmetic on the elements of the pairs. As a result, I suspect we really just want to store the two double-precision numbers, and essentially delegate to APFloat IEEEdouble in the implementation of the various operations.

As such, one option might be that, when PPCDoubleDouble fltSemantics are selected, the exponent and sign are ignored, and we just store the data for the two double-precision floating-point numbers in the significand. We might even do this directly, by making the significand something like this:

union Significand {
integerPart part;
integerPart *parts;
APFloat *fltparts; // used for PPCDoubleDouble
} significand;

We can do this, but my concern is “what’s next?”. What do we do to significandParts()? It doesn’t make sense to return an array of integerParts for PPCDoubleDouble. Do we examine every call site of significandParts(), and assert/dispatch on fltSemantics? Do we want to build an abstraction between APFloat and the underlying data representation? For example:

class APFloatPayload {
union Significand { integerPart part; integerPart *parts; } significand;
int exponent;

public:

// *Significand() operations.
};

class APFloat {
fltSemantics *semantics;

fltCategory category;

APFloatPayload first;
Optional second; // for PPCDoubleDouble
};

From: "Tim Shen" <timshen@google.com>
To: "Hal Finkel" <hfinkel@anl.gov>
Cc: resistor@mac.com, "Fiona Glaser" <escha@apple.com>, "Stephen
Canon" <scanon@apple.com>, "Eric Christopher" <echristo@gmail.com>,
llvm-dev@lists.llvm.org
Sent: Monday, October 3, 2016 1:27:12 PM
Subject: Re: [PPC, APFloat] Add full PPCDoubleDouble to APFloat

Hi Hal,

> Hi Tim,

> How, in general, are you thinking about doing this? I ask because,
> as
> you clearly know, the double-double format is formed by the sum of
> two double-precision numbers, and the various arithmetic operations
> are formed mostly in terms of double-precision arithmetic on the
> elements of the pairs. As a result, I suspect we really just want
> to
> store the two double-precision numbers, and essentially delegate to
> APFloat IEEEdouble in the implementation of the various operations.

> As such, one option might be that, when PPCDoubleDouble
> fltSemantics
> are selected, the exponent and sign are ignored, and we just store
> the data for the two double-precision floating-point numbers in the
> significand. We might even do this directly, by making the
> significand something like this:

> union Significand {

> integerPart part;

> integerPart *parts;

> APFloat *fltparts; // used for PPCDoubleDouble

> } significand;

We can do this, but my concern is "what's next?". What do we do to
significandParts()? It doesn't make sense to return an array of
integerParts for PPCDoubleDouble. Do we examine every call site of
significandParts(), and assert/dispatch on fltSemantics?

Indeed, this is why I was asking if you had an idea of how you were going to implement the arithmetic operations. It would be somewhat unfortunate if all of the functions essentially had the form of:

add(APFoat &RHS) {
if (semantic == PPCDoubleDouble)
return addPPCDoubleDouble(RHS);

...
}

but it might be the best option nonetheless (I can't think of anything better).

Do we want to build an abstraction between APFloat and the underlying
data representation? For example:

class APFloatPayload {
union Significand { integerPart part; integerPart *parts; }
significand;
int exponent;

public:

// *Significand() operations.
};

class APFloat {
fltSemantics *semantics;

fltCategory category;
APFloatPayload first;
Optional<APFloatPayload> second; // for PPCDoubleDouble
};

There might be some tradeoff here between performance and maintainability. APFloat already isn't *fast*, so I'm not super worried about introducing some extra checks inside the functions, but I'm more worried about increasing the size of the APFloat object itself, because we might have *a lot* of these in memory at once. As a result, I'd prefer a solution that did not increase the size of the base APFloat object (because then everyone pays a price for this and most people won't use it).

Thanks again,
Hal

There might be some tradeoff here between performance and maintainability.

I don’t think that it’s a tradeoff. We can have both. For example:

struct IEEEPayload {
union Significand { integerPart part; integerPart *parts; } significand;
int exponent;
int category : 3;
int sign : 1;
};

struct PPCDoubleDoublePayload {
struct {
int exponent1, exponent2;
integerPart significands; // flexible array member, or llvm::TrailingObjects instead.
} *heapdata;

int category : 3;

int sign : 2;

};

class APFloat {
fltSemantics *semantics;
union {
IEEEPayload ieee;
PPCDoubleDoublePayload ppc;
} payload;
};

It’s just an example of how we can use the same shallow space.

My previous point is that, I want to build an internal abstraction in the hope to reduce technical debt and maintenance burden, but I\we need to do it correctly.

From: "Tim Shen" <timshen@google.com>
To: "Hal Finkel" <hfinkel@anl.gov>
Cc: resistor@mac.com, "Fiona Glaser" <escha@apple.com>, "Stephen
Canon" <scanon@apple.com>, "Eric Christopher" <echristo@gmail.com>,
llvm-dev@lists.llvm.org
Sent: Monday, October 3, 2016 3:52:12 PM
Subject: Re: [PPC, APFloat] Add full PPCDoubleDouble to APFloat

> There might be some tradeoff here between performance and
> maintainability.

I don't think that it's a tradeoff. We can have both. For example:

struct IEEEPayload {
union Significand { integerPart part; integerPart *parts; }
significand;
int exponent;
int category : 3;
int sign : 1;
};

struct PPCDoubleDoublePayload {
struct {
int exponent1, exponent2;
integerPart significands; // flexible array member, or
llvm::TrailingObjects instead.
} *heapdata;

int category : 3;

int sign : 2;

};

class APFloat {
fltSemantics *semantics;
union {
IEEEPayload ieee;
PPCDoubleDoublePayload ppc;
} payload;
};

It's just an example of how we can use the same shallow space.

My previous point is that, I want to build an internal abstraction in
the hope to reduce technical debt and maintenance burden, but I\we
need to do it correctly.

That looks good to me (using llvm::TrailingObjects seems a bit cleaner than the trailing array).

-Hal