676721 - IM: Simple test case no longer works with lsra, causes segfault with greedy-ra

Status: RESOLVED DUPLICATE of bug 677066

(Core :: JavaScript Engine, defect)

Description

[Andrew Scheff]

The following test case prints 0 (should print 1) when I run with lsra and segfaults when I run with greedy register allocation.  This could absolutely be a problem with my latest patch, but I don't think it is because this test case has no branching! Can someone else try running this?

function f(a, b) {
    return a & b;
}

print(f(3, 5))

Comment 1

[Andrew Drake [:adrake]]

Prints 1 for me on ionmonkey tip with "js --ion testcase.js" on 32 bit Linux debug builds with no patches applied, but prints 0 for me on 64 bit.

Looks like the LSRA side of this is caused by the assumption that temporary allocations are only required for the output half of an instruction, which appears not to be the case for BOXes.

Working on a fix for the LSRA side.

Comment 2

[Andrew Drake [:adrake]]

Attachment: Patch v0

Fixes a couple issues:

1) Linear scan allocator allocated temporaries on top of the input registers to instructions.
2) The x86/x64 trampolines don't set their return values explicitly, so enterJIT can spuriously return false.

I wasn't able to reproduce the segfault issue after a clobber build.

Comment 3

[Andrew Scheff]

With 64 bit build, I still get 0 with lsra and 1 with greedy.  Segfault and everything else is fixed

Comment 4

[Andrew Drake [:adrake]]

Attachment: Patch 2 v0

BOX also clobbers its output registers before reading its input registers, an invariant I didn't realize was present. This corrects this assumption, and the test case now prints 1 for me with lsra enabled.

Comment 5

[Andrew Drake [:adrake]]

http://hg.mozilla.org/projects/ionmonkey/rev/bad4742bde20
http://hg.mozilla.org/projects/ionmonkey/rev/b35d66c93b8b

Comment 6

[Andrew Scheff]

I'm getting similar behavior now with the same test case.  With lsra I get the following assert:

Assertion failure: live->empty(), at /Users/Andy/ionmonkey/js/src/ion/LinearScan.cpp:475

With greedy ra it prints 0.. I'll look into this one

This is with a 64 bit build

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

New assertion could be related to bug 677314

Comment 8

[Andrew Drake [:adrake]]

New assertion is bug 677066. The graph is invalid at this point, so neither register allocator can do anything sane, the greedy allocator just doesn't notice because it doesn't do liveness.