Closed Bug 1074833 Opened 9 years ago Closed 9 years ago

Scalar Replacement: Do not mutate starting-block loop header.

Categories

(Core :: JavaScript Engine: JIT, defect)

Product:
Core
Component:
JavaScript Engine: JIT
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla35

People

(Reporter: nbp, Assigned: nbp)

References

Details

Attachments

(1 file, 1 obsolete file)

Scalar Replacement should not consider the starting block as a valid successors.
3.03 KB, patch
h4writer
: review+
Details | Diff | Splinter Review 
See tests cases from bug 1069307 comment 7.

The loop header can be the starting block of the escape analysis, thus we are trying to substitute the arguments of the initial state of the object and interpret each element as a Phi.
No longer depends on: 1072911
Delta:
 - Add test cases.
Attachment #8497508 - Attachment is obsolete: true
Attachment #8497508 - Flags: review?(hv1989)
Attachment #8497564 - Flags: review?(hv1989)
Comment on attachment 8497564 [details] [diff] [review]
Scalar Replacement should not consider the starting block as a valid successors.

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

lgtm, but it is quite strange to have the same code in mergeIntoSuccessorState for ObjectMemoryView and ArrayMemoryView. Somehow this should have been merged to have one function.
Attachment #8497564 - Flags: review?(hv1989) → review+
(In reply to Hannes Verschore [:h4writer] from comment #3)
> Comment on attachment 8497564 [details] [diff] [review]
> Scalar Replacement should not consider the starting block as a valid
> successors.
> 
> Review of attachment 8497564 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> lgtm, but it is quite strange to have the same code in
> mergeIntoSuccessorState for ObjectMemoryView and ArrayMemoryView. Somehow
> this should have been merged to have one function.

Merging this function implies adding an iterator to the M*State structures, and it does not fit the AliasAnalysis iteration.
https://hg.mozilla.org/mozilla-central/rev/6ebee67371f4
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.