Closed Bug 676721 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 677066

People

(Reporter: ascheff, Assigned: adrake)

Details

Attachments

(2 files)

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))
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.
Attached patch Patch v0Splinter Review
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.
Assignee: general → adrake
Status: NEW → ASSIGNED
Attachment #550916 - Flags: review?(dvander)
Attachment #550916 - Flags: review?(dvander) → review+
With 64 bit build, I still get 0 with lsra and 1 with greedy.  Segfault and everything else is fixed
Attached patch Patch 2 v0Splinter Review
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.
Attachment #551182 - Flags: review?(dvander)
Attachment #551182 - Flags: review?(dvander) → review+
http://hg.mozilla.org/projects/ionmonkey/rev/bad4742bde20
http://hg.mozilla.org/projects/ionmonkey/rev/b35d66c93b8b
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
New assertion could be related to bug 677314
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.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 677066
You need to log in before you can comment on or make changes to this bug.