...
Created attachment 353687 [details] Patch
Comment on attachment 353687 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353687&action=review > LayoutTests/js/script-tests/regress-139548.js:1 > -// FIXME: unskip when this is solved > -// https://bugs.webkit.org/show_bug.cgi?id=191163 > -//@ skip if $architecture == "mips" or $architecture == "arm" > +//@ skip if not $jitTests FWIW, I think all the occurrences of this make sense, + then we won't have to manually unskip tests when we reactivate the JIT. The reason I did not do that in the first place is that I did not know whether other platforms were faster and wouldn't need the skipping, so I tried to minimize the skipping. But if it's established that it's needed for all platforms when JIT is disabled, then I think that's how we should do it. > JSTests/microbenchmarks/array-push-1.js:1 > +//@ skip if not $jitTests All the tests that were not previously skipped on arm or mips do manage to run within the timeout limit on the mips and arm bots, (except stress/sampling-profiler-richards.js which I forgot to skip on mips), so I think it could make sense to *not* skip them on these platforms, which would give us more test coverage of these platforms (currently on cloop) for the time being. > JSTests/stress/op_lshift-ConstVar.js:1 > -// FIXME: unskip when this is solved > -// https://bugs.webkit.org/show_bug.cgi?id=191163 > -//@ skip if $architecture == "arm" > +//@ skip if not $jitTests Similarly, the tests that I skipped only on arm run within the timeout limit on mips. Though arguably we could lower the timeout limit on our mips bot.
Comment on attachment 353687 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353687&action=review r=me > Source/JavaScriptCore/ChangeLog:15 > + REGRESSION(r237547): Exception handlers should be aware of wide opcodes when JIT is disabled Remove this entry from the changelog since it’s for a different bug. > Source/JavaScriptCore/jit/JITExceptions.cpp:76 > + catchRoutine = catchPCForInterpreter->isWide() I think this already landed?
Created attachment 353843 [details] Patch
Comment on attachment 353687 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353687&action=review >> Source/JavaScriptCore/jit/JITExceptions.cpp:76 >> + catchRoutine = catchPCForInterpreter->isWide() > > I think this already landed? Oops, I had an early commit in the tree and didn't notice. I removed it now. >> JSTests/microbenchmarks/array-push-1.js:1 >> +//@ skip if not $jitTests > > All the tests that were not previously skipped on arm or mips do manage to run within the timeout limit on the mips and arm bots, (except stress/sampling-profiler-richards.js which I forgot to skip on mips), so I think it could make sense to *not* skip them on these platforms, which would give us more test coverage of these platforms (currently on cloop) for the time being. Sounds good, I changed all of this to only skip on x86. My rational was that most of these tests don't seem to do a whole lot of testing if the JIT is disabled, but I'm happy to un-skip them. >> JSTests/stress/op_lshift-ConstVar.js:1 >> +//@ skip if not $jitTests > > Similarly, the tests that I skipped only on arm run within the timeout limit on mips. Though arguably we could lower the timeout limit on our mips bot. I've changed those to skip only on arm || x86. I've also added back the FIXME comments.
Created attachment 354066 [details] Patch for landing
Comment on attachment 354066 [details] Patch for landing Clearing flags on attachment: 354066 Committed r237919: <https://trac.webkit.org/changeset/237919>
All reviewed patches have been landed. Closing bug.
<rdar://problem/45868793>