Ticket #1813 (closed bug: done)

Opened 4 years ago

Last modified 3 years ago

t/op/infnan.t: failures on Darwin/PPC

Reported by: jkeenan Owned by: plobsing
Priority: normal Milestone:
Component: core Version: 2.8.0
Severity: medium Keywords:
Cc: Language:
Patch status: Platform: darwin

Description

Somewhere between r49383 and r49432, we began to experience failures in t/op/infnan.t.

$ prove -v t/op/inf_nan.t 
t/op/inf_nan.t .. 
1..37
not ok 1 - basic arithmetic: =
# Have: inf
# Want: Inf
ok 2 - ... -=
ok 3 - ... *= -1
ok 4 - ... *= 0
ok 5 - ... += 5
ok 6 - ... -= 42
ok 7 - ... inc
ok 8 - ... dec
ok 9 - ... abs NaN
not ok 10 - ... abs Inf
# Have: inf
# Want: Inf
not ok 11 - ... abs -Inf
# Have: inf
# Want: Inf
not ok 12 - sqrt: assignment
# Have: inf
# Want: Inf
ok 13 - ... sqrt -Inf
ok 14 - ... sqrt NaN
ok 15 - ... sqrt -1
not ok 16 - negative: neg Inf
# Have: -inf
# Want: -Inf
not ok 17 - ... neg -Inf
# Have: inf
# Want: Inf
ok 18 - ... neg NaN
ok 19 - mixing NaN and Inf: NaN * Inf
ok 20 - ... NaN / Inf
ok 21 - ... NaN - Inf
ok 22 - ... NaN + Inf
ok 23 - rounding n: floor NaN
ok 24 - ... ceil NaN
not ok 25 - ... floor Inf
# Have: inf
# Want: Inf
not ok 26 - ... ceil Inf
# Have: inf
# Want: Inf
not ok 27 - ... floor -Inf
# Have: -inf
# Want: -Inf
not ok 28 - ... ceil -Inf
# Have: -inf
# Want: -Inf
ok 29 #skip rounding nan/inf gives something like -2147483648
ok 30 #skip rounding nan/inf gives something like -2147483648
ok 31 #skip rounding nan/inf gives something like -2147483648
ok 32 #skip rounding nan/inf gives something like -2147483648
ok 33 #skip 1+i + NaN should be NaN
ok 34 #skip fdiv/mod do not play nicely with PMCs and NaN
ok 35 #skip fdiv/mod do not play nicely with PMCs and NaN
ok 36 #skip fdiv/mod do not play nicely with PMCs and NaN
ok 37 #skip fdiv/mod do not play nicely with PMCs and NaN
Failed 10/37 subtests 
        (less 9 skipped subtests: 18 okay)

Test Summary Report
-------------------
t/op/inf_nan.t (Wstat: 0 Tests: 37 Failed: 10)
  Failed tests:  1, 10-12, 16-17, 25-28
Files=1, Tests=37,  0 wallclock secs ( 0.14 usr  0.04 sys +  0.06 cusr  0.04 csys =  0.28 CPU)
Result: FAIL

Since a new configuration step, auto::infnan, was added at r49425, I suspect that was the revision where these test failures emerged.

Note: AFAICT there was no discussion on list about adding this new configuration step, nor was there a Trac ticket presenting a rationale for this new configuration step. The new step was committed directly to trunk without prior development in a branch. I thought we had gotten away from this style of work.

This is not a good development practice, as it prevents Parrot developers other than the committer from:

(a) evaluating the rationale for the new step -- is it needed at all? is this the best approach?

(b) testing the code on our normal set of platforms.

Thank you very much.

kid51

Attachments

infnan_cast.patch Download (0.9 KB) - added by plobsing 4 years ago.

Change History

follow-ups: ↓ 2 ↓ 4   Changed 4 years ago by plobsing

What I suspect is happening is that the check against infinity in src/spf_render.c:Parrot_sprintf_format is failing, and the system sprintf is rendering the number as 'inf'.

Assuming IEEE754, pos/neg inf only have one representation each. This would suggest some sort of failure to extend infinites for comparisons.

Please test the attached patch which adds casts to the comparisons.

Changed 4 years ago by plobsing

in reply to: ↑ 1   Changed 4 years ago by plobsing

Replying to plobsing:

What I suspect is happening is that the check against infinity in src/spf_render.c:Parrot_sprintf_format is failing, and the system sprintf is rendering the number as 'inf'. Assuming IEEE754, pos/neg inf only have one representation each. This would suggest some sort of failure to extend infinites for comparisons. Please test the attached patch which adds casts to the comparisons.

What we got from a short debug session on irc:

  • the patch does not work
  • PARROT_HAS_INF_NAN is defined
  • i386 (working), and ppc (non-working) versions of math.h have identical definintions of INFINITY

Avenues of attack:

  • is the value contained in the HUGEFLOATVAL an infinity at the point of comparison?
> cat test.pir
.sub 'main' :main
    $N0 = 'Inf'
    say $N0
.end
> gdb --args ./parrot test.pir
...
(gdb) break src/spf_render.c:833
...
Breakpoint 1 (src/spf_render.c:833) pending.
(gdb) r
...
(gdb) p thefloat 
$1 = inf

  • is the comparison operation being performed valid? Snip out the relevant portion (comparisons with PARROT_FLOATVAL_INF* around src/spf_render.c:828) out of:
> gdb blib/lib/libparrot.so
...
(gdb) disassemble/m Parrot_sprintf_format

follow-up: ↓ 5   Changed 4 years ago by plobsing

Status Update:

  • the responsible changes were reverted on trunk and moved to branches/infnan
  • the affected test files are t/op/inf_nan.t, t/dynoplibs/trans-infnan.t, and t/op/string.t
  • this problem is only manifesting on darwin/ppc/g++. gcc does not exhibit this problem

in reply to: ↑ 1   Changed 4 years ago by doughera

Replying to plobsing:

What I suspect is happening is that the check against infinity in src/spf_render.c:Parrot_sprintf_format is failing, and the system sprintf is rendering the number as 'inf'. Assuming IEEE754, pos/neg inf only have one representation each. This would suggest some sort of failure to extend infinites for comparisons.

Would it make sense to use isinf() (if it's available and works for parrot's FLOATVALS) instead of trying to do a comparison?

in reply to: ↑ 3 ; follow-up: ↓ 7   Changed 4 years ago by jkeenan

Replying to plobsing:

Status Update: * the responsible changes were reverted on trunk and moved to branches/infnan * the affected test files are t/op/inf_nan.t, t/dynoplibs/trans-infnan.t, and t/op/string.t * this problem is only manifesting on darwin/ppc/g++. gcc does not exhibit this problem

Unfortunately, I cannot confirm that the source of these test failures is the use of g++. I have gotten the same pattern of test failures on Darwin/PPC with three different configurations.

First, my 'standard' approach, i.e., the same one I've used on this box since late 2006:

CC="/usr/bin/gcc"
CX="/usr/bin/g++"
/usr/local/bin/perl Configure.pl --cc="$CC" \
    --cxx="$CX" --link="$CX" \
    --ld="$CX" \
    --configure_trace \
    $@

This uses g++ for --link and --ld. This led to the errors reported originally in this TT as well as TT #1814 and TT #1815.

Second, stripping away all the command-line options:

perl Configure.pl

This results in the same failures. This is not really surprising because, in the absence of command-line options, Configure.pl will make its selections for --link and --ld on the basis of the %Config from the Perl used to invoke Configure.pl. And, on this box, once I follow all the symbolic links, you end up with g++ for --link and --ld just as I got in the first case.

Third: I set --link and --ld on the command line to gcc, thereby overriding (I think) what Configure.pl would normally default to.

perl Configure.pl  --cc=/usr/bin/gcc --cxx=/usr/bin/g++ \
> --link=/usr/bin/gcc --ld=/usr/bin/gcc --configure_trace

In other words, I purge g++ from the configuration and build as much as I know how to. In lib/Parrot/Config/Generated.pm, this leads to the following values:

  'ld' => '/usr/bin/gcc',
...
  'link' => '/usr/bin/gcc',

But I still get:

t/op/inf_nan.t                            
  (Wstat: 0 Tests: 37 Failed: 10)
  Failed tests:  1, 10-12, 16-17, 25-28
t/op/string.t                             
  (Wstat: 0 Tests: 291 Failed: 4)
  Failed tests:  280-283
t/dynoplibs/trans-infnan.t                
  (Wstat: 0 Tests: 70 Failed: 10)
  Failed tests:  1, 7-8, 18-19, 58, 61, 64, 67-68

... as reported in  Smolder 554.

So it's not, or not simply, g++. I wish I had better news to report.

Thank you very much.

kid51

  Changed 3 years ago by dukeleto

  • owner set to dukeleto
  • status changed from new to assigned

in reply to: ↑ 5 ; follow-up: ↓ 8   Changed 3 years ago by plobsing

  • status changed from assigned to new
  • owner changed from dukeleto to plobsing

Replying to jkeenan:

Replying to plobsing:

Status Update: * the responsible changes were reverted on trunk and moved to branches/infnan * the affected test files are t/op/inf_nan.t, t/dynoplibs/trans-infnan.t, and t/op/string.t * this problem is only manifesting on darwin/ppc/g++. gcc does not exhibit this problem

Unfortunately, I cannot confirm that the source of these test failures is the use of g++. I have gotten the same pattern of test failures on Darwin/PPC with three different configurations. First, my 'standard' approach, i.e., the same one I've used on this box since late 2006: {{{ CC="/usr/bin/gcc" CX="/usr/bin/g++" /usr/local/bin/perl Configure.pl --cc="$CC" \ --cxx="$CX" --link="$CX" \ --ld="$CX" \ --configure_trace \ $@ }}} This uses g++ for --link and --ld. This led to the errors reported originally in this TT as well as TT #1814 and TT #1815. Second, stripping away all the command-line options: {{{ perl Configure.pl }}} This results in the same failures. This is not really surprising because, in the absence of command-line options, Configure.pl will make its selections for --link and --ld on the basis of the %Config from the Perl used to invoke Configure.pl. And, on this box, once I follow all the symbolic links, you end up with g++ for --link and --ld just as I got in the first case. Third: I set --link and --ld on the command line to gcc, thereby overriding (I think) what Configure.pl would normally default to. {{{ perl Configure.pl --cc=/usr/bin/gcc --cxx=/usr/bin/g++ \

--link=/usr/bin/gcc --ld=/usr/bin/gcc --configure_trace

}}} In other words, I purge g++ from the configuration and build as much as I know how to. In lib/Parrot/Config/Generated.pm, this leads to the following values: {{{ 'ld' => '/usr/bin/gcc', ... 'link' => '/usr/bin/gcc', }}} But I still get: {{{ t/op/inf_nan.t (Wstat: 0 Tests: 37 Failed: 10) Failed tests: 1, 10-12, 16-17, 25-28 t/op/string.t (Wstat: 0 Tests: 291 Failed: 4) Failed tests: 280-283 t/dynoplibs/trans-infnan.t (Wstat: 0 Tests: 70 Failed: 10) Failed tests: 1, 7-8, 18-19, 58, 61, 64, 67-68 }}} ... as reported in  Smolder 554. So it's not, or not simply, g++. I wish I had better news to report. Thank you very much. kid51

Can you retest on origin/infnan2 branch? It makes use of math.h inf and nan test functions where available.

in reply to: ↑ 7   Changed 3 years ago by jkeenan

Replying to plobsing:

Can you retest on origin/infnan2 branch? It makes use of math.h inf and nan test functions where available.

So far, so good. I'm trying to test it in several variant configurations and will let you know when all are complete.

kid51

follow-up: ↓ 10   Changed 3 years ago by jkeenan

I have tested this branch in several different configurations on both Linux/i386 and Darwin/PPC. I get PASS all around on the test that was originally failing. Merge away! Thanks for your persistence in researching this.

kid51

in reply to: ↑ 9   Changed 3 years ago by plobsing

  • status changed from new to closed
  • resolution set to done

Replying to jkeenan:

I have tested this branch in several different configurations on both Linux/i386 and Darwin/PPC. I get PASS all around on the test that was originally failing. Merge away! Thanks for your persistence in researching this. kid51

Merged in cc69a53.

Note: See TracTickets for help on using tickets.