Closed Bug 778948 Opened 7 years ago Closed 6 years ago

move jsinterp.cpp/h to vm/

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: Benjamin, Unassigned)

Details

(Whiteboard: [js:t])

Attachments

(1 file, 4 obsolete files)

Now that we have the vm/ directly, we should probably put the actual virtual machine in there.
Summary: move jsinterp.ccp/h to vm/ → move jsinterp.cpp/h to vm/
Whiteboard: [js:t]
Attached patch move (obsolete) — Splinter Review
Attachment #674908 - Flags: review?(luke)
Seems fine to me; seem right to you guys?
Comment on attachment 674908 [details] [diff] [review]
move

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

I like how jsinterp.cpp doesn't itself include jsinterp.h, if this patch actually works.  :-D

::: js/src/Makefile.in
@@ +870,5 @@
>  	    javascript-trace.h.in > javascript-trace.h
>  
>  # We can't automatically generate dependencies on auto-generated headers;
>  # we have to list them explicitly.
> +$(addsuffix .$(OBJ_SUFFIX),jsprobes jsinsterp jsobj): $(CURDIR)/javascript-trace.h

Uh...

::: js/src/vm/ObjectImpl.cpp
@@ +18,5 @@
>  
>  #include "gc/Barrier-inl.h"
>  
>  #include "ObjectImpl-inl.h"
> +#include "Interpreter-inl.h"

Other order.
Attachment #674908 - Flags: feedback+
Attached patch move (obsolete) — Splinter Review
Waldo nits...
Attachment #674908 - Attachment is obsolete: true
Attachment #674908 - Flags: review?(luke)
Attachment #674991 - Flags: review?(luke)
Attachment #674991 - Flags: feedback?(dvander)
Comment on attachment 674991 [details] [diff] [review]
move

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

No arguments here. JSC has a separate "interpreter" folder, but our jsinterp.h has so many shared functions it might as well be part of vm/
Attachment #674991 - Flags: feedback?(dvander) → feedback+
Comment on attachment 674991 [details] [diff] [review]
move

Righto.
Attachment #674991 - Flags: review?(luke) → review+
Dave, does the fact that I only see the regression on one AWFY machine mean that it's some compiler bug? The Talos versions of sunspider don't seem to have regressed, and I can't reproduce locally.
Attached patch latest patch (obsolete) — Splinter Review
Attachment #674991 - Attachment is obsolete: true
Attached patch rebased (obsolete) — Splinter Review
Attachment #675811 - Attachment is obsolete: true
Attached patch rebased againSplinter Review
David, could you see what's happening on AWFY?

This Try build shows it doesn't regress TBPL anymore:
https://tbpl.mozilla.org/?tree=Try&rev=6274db32264e
Attachment #677571 - Attachment is obsolete: true
Attachment #683114 - Flags: feedback?(dvander)
Attachment #683114 - Flags: feedback?(dvander)
https://hg.mozilla.org/mozilla-central/rev/5a21e83107b0
https://hg.mozilla.org/mozilla-central/rev/3025f175e2f1
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.