The default bug view has changed. See this FAQ.

IonMonkey: Simplify the greedy allocator

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dvander, Assigned: dvander)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 582585 [details] [diff] [review]
WIP v0

I'm still not sure whether this allocator is useful, but it's really complex (arguably, moreso than LSRA) and has lots of bugs. This patch simplifies it by removing all the control flow handling code and instead spilling the register state at the top of basic blocks. It still gets good allocation inside a basic block.

(Note, this patch is totally untested.)
(Assignee)

Comment 1

5 years ago
Created attachment 583034 [details] [diff] [review]
patch

 7 files changed, 212 insertions(+), 624 deletions(-)

This also fixes a bunch of test failures, so I guess the old implementation was actually pretty buggy.
Attachment #583034 - Flags: review?(sstangl)
(Assignee)

Comment 2

5 years ago
Created attachment 583331 [details] [diff] [review]
v2

This version is a little less aggressive. It keeps FindNaturalLoops which makes safepoint integration much easier.
Attachment #582585 - Attachment is obsolete: true
Attachment #583034 - Attachment is obsolete: true
Attachment #583034 - Flags: review?(sstangl)
Attachment #583331 - Flags: review?(sstangl)
(Assignee)

Updated

5 years ago
Duplicate of this bug: 706896
(Assignee)

Updated

5 years ago
Duplicate of this bug: 708757
Comment on attachment 583331 [details] [diff] [review]
v2

Review of attachment 583331 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great. It's significantly simpler. Haven't found any issues.

::: js/src/ion/C1Spewer.cpp
@@ +55,5 @@
>  
>  bool
>  C1Spewer::init(const char *path)
>  {
> +    spewout_ = NULL; //fopen(path, "w");

Hm.

::: js/src/ion/MIRGraph.h
@@ +406,5 @@
>          JS_ASSERT(isLoopHeader());
>          return containedInLoop_.append(block);
>      }
>  
> +	void setLoopDepth(uint32 loopDepth) {

One indentation level back
Attachment #583331 - Flags: review?(sstangl) → review+
(Assignee)

Comment 6

5 years ago
http://hg.mozilla.org/projects/ionmonkey/rev/3725545a3e35
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.