WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
209964
Re-enable previously skipped mips tests
https://bugs.webkit.org/show_bug.cgi?id=209964
Summary
Re-enable previously skipped mips tests
Paulo Matos
Reported
2020-04-03 07:19:41 PDT
Re-enable previously skipped mips tests
Attachments
Patch
(5.26 KB, patch)
2020-04-03 07:21 PDT
,
Paulo Matos
no flags
Details
Formatted Diff
Diff
Patch
(6.17 KB, patch)
2020-04-14 04:42 PDT
,
Paulo Matos
no flags
Details
Formatted Diff
Diff
Patch
(6.11 KB, patch)
2020-04-15 03:56 PDT
,
Paulo Matos
no flags
Details
Formatted Diff
Diff
Patch
(6.12 KB, patch)
2020-04-16 01:11 PDT
,
Paulo Matos
no flags
Details
Formatted Diff
Diff
Patch
(5.26 KB, patch)
2020-04-16 01:59 PDT
,
Paulo Matos
no flags
Details
Formatted Diff
Diff
Patch
(5.27 KB, patch)
2020-04-16 23:27 PDT
,
Paulo Matos
no flags
Details
Formatted Diff
Diff
Patch
(5.26 KB, patch)
2020-04-17 05:58 PDT
,
Paulo Matos
no flags
Details
Formatted Diff
Diff
Patch
(5.27 KB, patch)
2020-04-17 06:02 PDT
,
Paulo Matos
no flags
Details
Formatted Diff
Diff
Show Obsolete
(7)
View All
Add attachment
proposed patch, testcase, etc.
Paulo Matos
Comment 1
2020-04-03 07:21:08 PDT
Created
attachment 395373
[details]
Patch
David Kilzer (:ddkilzer)
Comment 2
2020-04-03 11:39:11 PDT
Comment on
attachment 395373
[details]
Patch cq- since `jsc` tests are failing. Can't land with tests failing. <
https://ews-build.webkit.org/#/builders/1/builds/10294
>
Paulo Matos
Comment 3
2020-04-14 04:42:45 PDT
Created
attachment 396400
[details]
Patch
Paulo Matos
Comment 4
2020-04-15 03:56:01 PDT
Created
attachment 396518
[details]
Patch
David Kilzer (:ddkilzer)
Comment 5
2020-04-15 17:35:33 PDT
Comment on
attachment 396518
[details]
Patch This seems fine now, but I'd prefer to let a JSC expert review in case there's something subtle I'm missing.
Mark Lam
Comment 6
2020-04-15 23:38:17 PDT
Comment on
attachment 396518
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=396518&action=review
> JSTests/stress/generator-cell-with-type.js:2 > +//@ requireOptions("-e", "let hardness=1e5") if ["mips"].include?($architecture)
Is there a reason the following wouldn't work? //@ requireOptions("-e 'let hardness=1e5'") if ["mips"].include?($architecture) I expect requireOptions(x, y, z) to read as the test needing options x, y, and z. However, the way you have it above reads like requiring option "-e" and option "let hardness=1e5", which I think is weird. Would be nice if we can avoid this.
> JSTests/stress/generator-cell-with-type.js:4 > +hardness = typeof(hardness) === 'undefined' ? 1e6 : hardness;
Let's just call this "iterations" instead "hardness".
Paulo Matos
Comment 7
2020-04-16 01:00:07 PDT
(In reply to Mark Lam from
comment #6
)
> Comment on
attachment 396518
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=396518&action=review
> > > JSTests/stress/generator-cell-with-type.js:2 > > +//@ requireOptions("-e", "let hardness=1e5") if ["mips"].include?($architecture) > > Is there a reason the following wouldn't work? > //@ requireOptions("-e 'let hardness=1e5'") if > ["mips"].include?($architecture) >
That was the first thing I tried but it doesn't work: Could not open file: -e 'let hardness=1e5' Unfortunately when the script calls the command it passes the whole thing as a single argument and jsc seems to think it's a file. I looked into this earlier on when committed this
r259114
.
> I expect requireOptions(x, y, z) to read as the test needing options x, y, > and z. However, the way you have it above reads like requiring option "-e" > and option "let hardness=1e5", which I think is weird. Would be nice if we > can avoid this. > > > JSTests/stress/generator-cell-with-type.js:4 > > +hardness = typeof(hardness) === 'undefined' ? 1e6 : hardness; > > Let's just call this "iterations" instead "hardness".
Sure - I will push a new patch straight away.
Paulo Matos
Comment 8
2020-04-16 01:11:33 PDT
Created
attachment 396628
[details]
Patch
Paulo Matos
Comment 9
2020-04-16 01:59:38 PDT
Created
attachment 396629
[details]
Patch
Mark Lam
Comment 10
2020-04-16 11:02:06 PDT
Comment on
attachment 396629
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=396629&action=review
I noticed that you removed the change in jsc-run-stress-tests that added quoting around options with white spaces. Did that turn out to not be needed after all?
> JSTests/stress/generator-cell-with-type.js:2 > +//@ requireOptions("-e", "let hardness=1e5") if ["mips"].include?($architecture)
Why keep the hardness name at all? What does "hardness" mean? What's happening here is that you're trying to override the iterations. Why not just call it that?
> JSTests/stress/generator-cell-with-type.js:4 > +iterations = typeof(hardness) === 'undefined' ? 1e6 : hardness;
Ditto.
Paulo Matos
Comment 11
2020-04-16 23:24:47 PDT
(In reply to Mark Lam from
comment #10
)
> Comment on
attachment 396629
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=396629&action=review
> > I noticed that you removed the change in jsc-run-stress-tests that added > quoting around options with white spaces. Did that turn out to not be > needed after all? >
Correct. That was for bytecode-cache tests, which unfortunately cannot run when we pass options with -e, so I removed it.
> > JSTests/stress/generator-cell-with-type.js:2 > > +//@ requireOptions("-e", "let hardness=1e5") if ["mips"].include?($architecture) > > Why keep the hardness name at all? What does "hardness" mean? What's > happening here is that you're trying to override the iterations. Why not > just call it that?
> Ah - from your previous comment, I thought you wanted me to use hardness and iterations instead of just hardness. I understand now you just want to use iterations. In the mind of a non-native english speaker it means how hard something is supported to be... in any case, happy to use iterations.
> > JSTests/stress/generator-cell-with-type.js:4 > > +iterations = typeof(hardness) === 'undefined' ? 1e6 : hardness; > > Ditto.
Sure.
Paulo Matos
Comment 12
2020-04-16 23:27:03 PDT
Created
attachment 396742
[details]
Patch
Paulo Matos
Comment 13
2020-04-17 05:58:35 PDT
Created
attachment 396759
[details]
Patch
Paulo Matos
Comment 14
2020-04-17 06:02:03 PDT
Created
attachment 396760
[details]
Patch
Mark Lam
Comment 15
2020-04-17 07:13:16 PDT
Comment on
attachment 396760
[details]
Patch r=me
EWS
Comment 16
2020-04-21 02:52:50 PDT
Committed
r260421
: <
https://trac.webkit.org/changeset/260421
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 396760
[details]
.
Radar WebKit Bug Importer
Comment 17
2020-04-21 02:53:15 PDT
<
rdar://problem/62098580
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug