Re: ArrayFunctionsTest::testDubiousArrayFormulasFODS fails on ppc64le (and s390x)

[Dan Horák <dan@xxxxxxxx>]

On Fri, 1 Sep 2023 11:29:23 +0200
Stephan Bergmann <sbergman@xxxxxxxxxx> wrote:

> On 9/1/23 10:37, Dan Horák wrote:
> > The "dubious array" tests are already being skipped for aarch64 due
> > known issue [1], shouldn't we just skip them for ppc64le and s390x as
> > well? Or should it be enabled only for "good" platforms, eg. x86_64?
> 
> I think either approach should be OK, whatever you find more convenient. 
>   One minor downside when adding additional conditionals to skip on 
> ppc64le and s390x to
> 
> > void ArrayFunctionsTest::testDubiousArrayFormulasFODS()
> > {
> >     //TODO: sc/qa/unit/data/functions/array/dubious/fods/linest.fods produces widely different
> >     // values when built with -ffp-contract enabled (-ffp-contract=on default on Clang 14,
> >     // -ffp-contract=fast default when building with optimizations on GCC) on at least aarch64:
> > #if !((defined __clang__ || defined __GNUC__) && defined __aarch64__)
> >     OUString aDirectoryURL
> >         = m_directories.getURLFromSrc(u"/sc/qa/unit/data/functions/array/dubious/fods/");
> >     recursiveScan(test::pass, "OpenDocument Spreadsheet Flat XML", aDirectoryURL,
> >             "com.sun.star.comp.filter.OdfFlatXml,,com.sun.star.comp.Calc.XMLOasisImporter,com.sun.star.comp.Calc.XMLOasisExporter,,,true",
> >             FODS_FORMAT_TYPE, SotClipboardFormatId::NONE, 0, false);
> > #endif
> > }
> 
> would be that the TODO comment makes it then sound like the issues on 
> ppc64le and s390x are also known to be due to -ffp-contract settings, 
> even if you would not verify that.

I have verified it's indeed caused by -ffp-contract. When set to "off",
the test passes (checked on ppc64le). But what's more interesting, that
the "off" setting also fixes a failure in
AddinFunctionsTest::testAddinFormulasFODS when dec2bin() is converting
negative numbers. dec2bin(-100) and dec2bin(-2) report Error:502
instead of doing the conversion ...

 
> So maybe the best thing is to turn this into a x86_64 only conditional, 
> and extend the TODO comment with something like "...on at least aarch64, 
> so lets only execute this on x86-64, where it appears to always work well:"

yeah, I think using a "positive" list would be better. I could
possibly prepare a patch, but is there an example where I can see how
the condition should look like?

> (The underlying issue is that tests like this are, excuse my wording, 
> utter crap.  Instead of checking for exact values, they should of course 
> check that results are in whatever acceptable range according to a spec. 
>   But Calc doesn't even come with such a spec in the first place...)

we have seen more problems in other packages in the past related to
-ffp-contract behaviour on aarch64 and s390x and ppc64le ...


		Dan