Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CArray[num64] numiness optimised away #4324

Open
p6rt opened this issue Jun 14, 2015 · 23 comments
Open

CArray[num64] numiness optimised away #4324

p6rt opened this issue Jun 14, 2015 · 23 comments
Labels

Comments

@p6rt
Copy link

p6rt commented Jun 14, 2015

Migrated from rt.perl.org#125408 (status was 'open')

Searchable as RT125408$

@p6rt
Copy link
Author

p6rt commented Jun 14, 2015

From @jonathanstowe

Hi,
The following code gives rise to​:

Wrong kind of access to numeric CArray
  in method ASSIGN-POS at lib/NativeCall.pm​:333
  in block at tp​:22

In this case it always appears after the 57th item and only when the array is in the third generation as here, it does not appear with MVM_SPESH_DISABLE=1 which lrads me to believe it is something the optimiser is doing.

I think nine did a simple bisect and located it to a few weeks ago.

use v6;

use NativeCall;

my @​buff := CArray[num64].new;

my @​in = (^100).map({Num($_)});

for ^100 -> $i {
  @​buff[$i] = @​in[$i];
}

my @​in2;

for ^100 -> $i {
  @​in2[$i] = @​buff[$i];
}

my @​buff2 := CArray[num64].new;

for ^100 -> $i {
  @​buff2[$i] = @​in2[$i];
}

@p6rt
Copy link
Author

p6rt commented Jun 14, 2015

From @jonathanstowe

I might add this one isn't speculative code, I'm actually using it in https://github.com/jonathanstowe/Audio-Sndfile - I've TODO'd the affected tests for the time being :)

On Sun Jun 14 14​:36​:08 2015, jns+bc@​gellyfish.co.uk wrote​:

Hi,
The following code gives rise to​:

Wrong kind of access to numeric CArray
in method ASSIGN-POS at lib/NativeCall.pm​:333
in block at tp​:22

In this case it always appears after the 57th item and only when the
array is in the third generation as here, it does not appear with
MVM_SPESH_DISABLE=1 which lrads me to believe it is something the
optimiser is doing.

I think nine did a simple bisect and located it to a few weeks ago.

use v6;

use NativeCall;

my @​buff := CArray[num64].new;

my @​in = (^100).map({Num($_)});

for ^100 -> $i {
@​buff[$i] = @​in[$i];
}

my @​in2;

for ^100 -> $i {
@​in2[$i] = @​buff[$i];
}

my @​buff2 := CArray[num64].new;

for ^100 -> $i {
@​buff2[$i] = @​in2[$i];
}

@p6rt
Copy link
Author

p6rt commented Jun 23, 2015

From @jonathanstowe

Hi,
it appears that the original code posted with this doesn't exhibit the problem any more as at "This is perl6 version 2015.06-42-gd078782 built on MoarVM version 2015.06-16-g46e941c" however the actual code that tickled this has got worse and it now affects num32s too (and confirm it is a related problem as the SPESH_DISABLE ... fixes it as before.)

I'll try and work up a new failing example.

@p6rt
Copy link
Author

p6rt commented Jun 23, 2015

@jonathanstowe - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Jun 24, 2015

From @jonathanstowe

A further data point, in the actual code this always fails on attempt to set the first element in a new CArray of type num32/num64.

The actual code does exactly what the example does, *except* that the initial CArray has been populated by a native function.

Still struggling to replicate outside the module.

If you want to see it's https://github.com/jonathanstowe/Audio-Sndfile

@p6rt
Copy link
Author

p6rt commented Jun 25, 2015

From @jonathanstowe

Right, managed to get something closer to what the actual code is doing, I hadn't noticed that the tests were all passing first time round the loop.

use NativeCall;

class Foo is repr("CStruct") {
  method foo(Mu​:U $type, Int $a ) {
  my @​buff := CArray[$type].new;
  my $ctype = $type ~~ Num ?? Num !! Int;
  @​buff[$_] = $ctype(0) for ^(10 * $a);
  }
}

for ^10 {
  say $_;
  for (int16, int32,num64, num32 ) -> $type {
  my $foo = Foo.new;
  $foo.foo($type, 10.rand.Int);
  }
}

@p6rt
Copy link
Author

p6rt commented Jun 25, 2015

From @jonathanstowe

Added the output of running the above with MVM_SPESH_LOG=broken_carray_access.txt (gzipped)

@p6rt
Copy link
Author

p6rt commented Jun 25, 2015

@p6rt
Copy link
Author

p6rt commented Jul 9, 2015

From @jonathanstowe

The latter example now seems to work fine at​:

This is perl6 version 2015.06-226-g01a60df built on MoarVM version 2015.06-88-g647df11

Also all the tests in Audio​::Sndfile which I had TODO'd as a result of this appear to be working fine now (as do the raft of failing tests in the not yet finished Audio​::Encode​::LameMP3 which is doing similar things.)

Not quite ready to close this until someone can identify what actually changed to fix it.

@p6rt
Copy link
Author

p6rt commented Jul 16, 2015

From @jonathanstowe

rakudo/rakudo#471 provides a test for this.

@p6rt
Copy link
Author

p6rt commented Jul 16, 2015

From @jonathanstowe

Canary test in for this, it passes. All good

@p6rt
Copy link
Author

p6rt commented Jul 16, 2015

@jonathanstowe - Status changed from 'open' to 'resolved'

@p6rt
Copy link
Author

p6rt commented Aug 3, 2015

From @coke

On Thu Jul 16 07​:58​:54 2015, jns+bc@​gellyfish.co.uk wrote​:

Canary test in for this, it passes. All good

This test is failing on OS X, reopening ticket.

This is perl6 version 2015.07.1-52-ga38b59c built on MoarVM version 2015.07-8-gb8fdeae

--
Will "Coke" Coleda

@p6rt
Copy link
Author

p6rt commented Aug 3, 2015

@coke - Status changed from 'resolved' to 'open'

@p6rt
Copy link
Author

p6rt commented Aug 11, 2015

From @jonathanstowe

This has come back again on Linux too with

This is perl6 version 2015.07.1-88-gaf75bd7 built on MoarVM version 2015.07-8-gb8fdeae

1 similar comment
@p6rt
Copy link
Author

p6rt commented Aug 11, 2015

From @jonathanstowe

This has come back again on Linux too with

This is perl6 version 2015.07.1-88-gaf75bd7 built on MoarVM version 2015.07-8-gb8fdeae

@p6rt
Copy link
Author

p6rt commented Aug 27, 2015

From paul@liekut.de

On Tue Aug 11 00​:26​:52 2015, jns+bc@​gellyfish.co.uk wrote​:

This has come back again on Linux too with

This is perl6 version 2015.07.1-88-gaf75bd7 built on MoarVM version
2015.07-8-gb8fdeae

This also turns up on Linux (Debian/jessie amd64) with​:

This is perl6 version 2015.07.1-164-ga7c845c built on MoarVM version 2015.07-68-g3240047.

After setting MVM_SPESH_DISABLE=1, the test passes again.

@p6rt
Copy link
Author

p6rt commented Sep 8, 2015

From @nwc10

On Thu Aug 27 01​:47​:46 2015, pcoch wrote​:

On Tue Aug 11 00​:26​:52 2015, jns+bc@​gellyfish.co.uk wrote​:

This has come back again on Linux too with

This is perl6 version 2015.07.1-88-gaf75bd7 built on MoarVM version
2015.07-8-gb8fdeae

This also turns up on Linux (Debian/jessie amd64) with​:

This is perl6 version 2015.07.1-164-ga7c845c built on MoarVM version
2015.07-68-g3240047.

After setting MVM_SPESH_DISABLE=1, the test passes again.

Bisecting reveals this commit to be the culprit​:

commit 56ae33ea0a5ac3d54a5fef2aa15eab34f9e9594b
Author​: jnthn <jnthn@​jnthn.net>
Date​: Fri Jul 31 13​:11​:41 2015 +0200

  Re-implement &?ROUTINE.
 
  This uses the same appraoch as &?BLOCK, meaning you only pay for the
  feature if you use it. This unbreaks MoarVM's ability to do a bunch of
  inlining, which has rather positive performance consequences.

src/Perl6/Actions.nqp | 54 +++++++++++++++++++++++++++++++--------------------
src/Perl6/World.nqp | 8 ++++++++
2 files changed, 41 insertions(+), 21 deletions(-)

No, it's not obvious why.

@p6rt
Copy link
Author

p6rt commented Sep 17, 2015

From @FROGGS

Fixed by jnthn++ with commit​:

MoarVM/MoarVM@60a48e8

@p6rt
Copy link
Author

p6rt commented Sep 17, 2015

@FROGGS - Status changed from 'open' to 'resolved'

@p6rt
Copy link
Author

p6rt commented Oct 23, 2015

From @coke

On Thu Sep 17 13​:37​:30 2015, FROGGS.de wrote​:

Fixed by jnthn++ with commit​:

MoarVM/MoarVM@60a48e8

This test file is failing on JVM on my box. reopening.

--
Will "Coke" Coleda

@p6rt
Copy link
Author

p6rt commented Oct 23, 2015

@coke - Status changed from 'resolved' to 'open'

@p6rt
Copy link
Author

p6rt commented Oct 24, 2015

From jns@gellyfish.co.uk

Good thing I didn't take the related TODO tests in my modules out :)

On Fri, 2015-10-23 at 11​:56 -0700, Will Coleda via RT wrote​:

On Thu Sep 17 13​:37​:30 2015, FROGGS.de wrote​:

Fixed by jnthn++ with commit​:

MoarVM/MoarVM@60a48e85b4928a9d66f2f8b87c5
fdb65633a7d99

This test file is failing on JVM on my box. reopening.

@p6rt p6rt added the Bug label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant