Closed Bug 685993 Opened 13 years ago Closed 13 years ago

IonMonkey: Propagate Phi specializations

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: dvander)

Details

Attachments

(1 file, 1 obsolete file)

On a mandlebrot benchmark, code looks like this:

  var x = 0;
  do {
    x = (floating point computation)
  } ...

Right now this creates a phi(int32, double), which produces a Value, and later unbox operations fail. We should propagate the double specialization up to the constant.
Attached patch fix (obsolete) — Splinter Review
This patch detects phis which has both int32 and double inputs, and forces them to specialize as doubles. Then it converts int32 inputs to doubles.

This works for the case in the benchmark, which no longer bails out. As more cases come up we might have to work on eliminating conversions by respecializing more.
Attachment #559625 - Flags: review?(sstangl)
Attached patch v2Splinter Review
Actual patch
Attachment #559625 - Attachment is obsolete: true
Attachment #559625 - Flags: review?(sstangl)
Attachment #559626 - Flags: review?(sstangl)
Comment on attachment 559626 [details] [diff] [review]
v2

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

It may be useful to have a folder or repository for internal benchmarks.

::: js/src/ion/IonAnalysis.cpp
@@ +297,5 @@
>      reanalyzeUses(phi);
>  }
>  
> +static inline bool
> +IsNumberType(MIRType type)

Could this exist as an inline in TypeOracle.h, where MIRType is defined?
Attachment #559626 - Flags: review?(sstangl) → review+
Pushed w/ nits:

http://hg.mozilla.org/projects/ionmonkey/rev/1cc1a573bb99
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.