[PATCH] D24011: [ConstantFold] Add a flag for ppc_fp128 constant folding, since APFloat doesn't support double-double semantic

[Hal Finkel via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D24011#528429, @timshen wrote:

> In https://reviews.llvm.org/D24011#528387, @majnemer wrote:
>
> > I'd be more comfortable with a patch that didn't attempt to fold in the face of PPCDoubleDouble without a flag to control it.
>
>
> We (me and iteratee) don't mind removing the flags. @hfinkel and @kbarton?


Agreed. We should either do this or not.

> 

> 

> In https://reviews.llvm.org/D24011#528401, @hfinkel wrote:

> 

> > In the past, we've been comfortable leaving this as an open known issue, especially given that the semantics of PPC long double are not IEEE anyway - the real fix involves creating an APFloat-based double-double implementation which can be used to evaluate constants.

> 

> 

> The problem this patch fixes is not the IEEE conformance, but the consistency. Currently libstdc++ `std::numeric_limits<long double>::epsilon()` returns an epsilon value that may be suitable for double-double semantic, but not suitable for the compile-time IEEE 128 semantic.


Yes, I completely understand that.

> 

> 

> > How important is it that we make this kind of change now?

> 

> 

> By "this kind of change" you mean being able to turn off constant folding for ppc_fp128? We observe internal test failures that due to the mix of using `std::numeric_limits<long double>::epsilon()` and LLVM constant folding, and we'd like to make the tests green, especially when they seem use the standard library correctly. :)


Yes. I'm certainly sympathetic to wanting correctness tests to pass. However, this has been a known issue in LLVM for nearly a decade, and we could have made this change at any time. We did not because we judged the benefit to be not worth the performance cost to applications that benefit from the constant folding. I'm not against doing this - I suspect few applications are depending on long-double calculations on PPC for high performance computation, but I think we need to understand the rationale in light of the history here.


https://reviews.llvm.org/D24011