1146295 - SIMD: a few cleanups

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Benjamin Bouvier [:bbouvier] (inactive)]

Those have been on my todo list for a while now...

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: MozReview Request: bz://1146295/bbouvier

/r/5841 - Bug 1146295: Remove "flags" macro argument in SIMD.h;
/r/5843 - Bug 1146295: Factor out SIMD types initialization;
/r/5845 - Bug 1146295: Factor out SIMD type's methods with their definitions class helper;
/r/5847 - Bug 1146295: SIMD types' definition helpers have nothing to do in the ::js namespace;
/r/5849 - Bug 1146295: Inline the last part of CreateAndBindSimdType into CreateAndBindSimdClass;
/r/5851 - Bug 1146295: Factor out SimdTypeDescr::call.
/r/5853 - Bug 1146295: Remove unused FuncZero function;
/r/5855 - Bug 1146295: Rename SimdTypeDescr::TYPE_SCALAR into SimdTypeDescr::TypeXn;

Pull down these commits:

hg pull review -r af3177adf567b8fead84300bf5672771a346a23b

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=286ece5aa7ca

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Oops, apparently tbpl doesn't like funcTemplate<::name>();
Maybe with a space...
https://treeherder.mozilla.org/#/jobs?repo=try&revision=836a7dea3134

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8581552 [details]
MozReview Request: bz://1146295/bbouvier

/r/5841 - Bug 1146295: Remove "flags" macro argument in SIMD.h;
/r/5843 - Bug 1146295: Factor out SIMD types initialization;
/r/5845 - Bug 1146295: Factor out SIMD type's methods with their definitions class helper;
/r/5847 - Bug 1146295: SIMD types' definition helpers have nothing to do in the ::js namespace;
/r/5849 - Bug 1146295: Inline the last part of CreateAndBindSimdType into CreateAndBindSimdClass;
/r/5851 - Bug 1146295: Factor out SimdTypeDescr::call.
/r/5853 - Bug 1146295: Remove unused FuncZero function;
/r/5855 - Bug 1146295: Rename SimdTypeDescr::TYPE_SCALAR into SimdTypeDescr::TypeXn;
/r/5963 - Bug 1146295: Make some arguments facultative in splat/with functions;

Pull down these commits:

hg pull review -r 6603f4c79e8afb2e40e541b31f88ea9e94fbeacf

Comment 5

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5963/#review5001

Comment 6

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5855/#review4999

Can you give some more context why this change is needed? What is wrong with the old naming?

Comment 7

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5853/#review4997

Comment 8

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5849/#review4993

Comment 9

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5843/#review4989

Comment 10

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5841/#review4987

Comment 11

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5851/#review4995

::: js/src/builtin/SIMD.cpp
(Diff revision 2)
> +FillLanes(JSContext *cx, MutableHandle<TypedObject*> result, const CallArgs &args)
> +{
> +    typedef typename T::Elem Elem;
> +    Handle<TypedObject*> handleResult(result);

Shouldn't 'result' already be a Handle<TypedObject *> instead of a MutableHandle. If I understand this code correctly it doesn't change the result object to another object...

Comment 12

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5845/#review4991

Comment 13

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8581552 [details]
MozReview Request: bz://1146295/bbouvier

/r/5841 - Bug 1146295: Remove "flags" macro argument in SIMD.h; r=h4writer
/r/5843 - Bug 1146295: Factor out SIMD types initialization; r=h4writer
/r/5845 - Bug 1146295: Factor out SIMD type's methods with their definitions class helper; r=h4writer
/r/5847 - Bug 1146295: SIMD types' definition helpers have nothing to do in the ::js namespace;
/r/5849 - Bug 1146295: Inline the last part of CreateAndBindSimdType into CreateAndBindSimdClass; r=h4writer
/r/5851 - Bug 1146295: Factor out SimdTypeDescr::call.
/r/5853 - Bug 1146295: Remove unused FuncZero function; r=h4writer
/r/5855 - Bug 1146295: Rename SimdTypeDescr::TYPE_SCALAR into SimdTypeDescr::TypeXn;
/r/5963 - Bug 1146295: Make some arguments facultative in splat/with functions; r=h4writer

Pull down these commits:

hg pull review -r 324b121cbd75111774f9e7ec3371686f15e3ab70

Comment 14

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5855/#review5215

Ship It!

Comment 15

[Benjamin Bouvier [:bbouvier] (inactive)]

https://reviewboard.mozilla.org/r/5855/#review5211

Just personal taste: it seems weird that SimdTypeDescr::TYPE_FLOAT32 (Scalar) actually means SimdTypeDescr::Float32x4. What do you think?

Comment 16

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5851/#review5217

Ship It!

Comment 17

[Hannes Verschore [:h4writer]]

https://reviewboard.mozilla.org/r/5847/#review5219

Ship It!

::: js/src/builtin/SIMD.cpp
(Diff revision 2)
> -namespace js {

Please use "anonymous namespace"

Comment 18

[Hannes Verschore [:h4writer]]

Comment on attachment 8581552 [details]
MozReview Request: bz://1146295/bbouvier

https://reviewboard.mozilla.org/r/5839/#review5221

Ship It!

Comment 19

[Benjamin Bouvier [:bbouvier] (inactive)]

Thanks!
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e78ddf91b230

Comment 20

[Benjamin Bouvier [:bbouvier] (inactive)]

Put aside the GC failures caused by a previous push, it's green, so i'll consider it green (at this point, this might be madness).

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/8db04357fe91
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bf1f3c86e45d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/cf48b5413dbc
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/742a3be51627
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d12be5ebcad5
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/2b3d979d974a
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/701309c14003
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/57bd743ef57b
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/98cf094bbc05

Comment 21

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8db04357fe91
https://hg.mozilla.org/mozilla-central/rev/bf1f3c86e45d
https://hg.mozilla.org/mozilla-central/rev/cf48b5413dbc
https://hg.mozilla.org/mozilla-central/rev/742a3be51627
https://hg.mozilla.org/mozilla-central/rev/d12be5ebcad5
https://hg.mozilla.org/mozilla-central/rev/2b3d979d974a
https://hg.mozilla.org/mozilla-central/rev/701309c14003
https://hg.mozilla.org/mozilla-central/rev/57bd743ef57b
https://hg.mozilla.org/mozilla-central/rev/98cf094bbc05

Comment 23

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: MozReview Request: Bug 1146295: Make some arguments facultative in splat/with functions; r=h4writer

Comment 24

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: MozReview Request: Bug 1146295: Remove "flags" macro argument in SIMD.h; r=h4writer

Comment 25

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: MozReview Request: Bug 1146295: Factor out SIMD types initialization; r=h4writer

Comment 26

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: MozReview Request: Bug 1146295: Factor out SIMD type's methods with their definitions class helper; r=h4writer

Comment 27

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: MozReview Request: Bug 1146295: SIMD types' definition helpers have nothing to do in the ::js namespace;

Comment 28

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: MozReview Request: Bug 1146295: Inline the last part of CreateAndBindSimdType into CreateAndBindSimdClass; r=h4writer

Comment 29

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: MozReview Request: Bug 1146295: Factor out SimdTypeDescr::call.

Comment 30

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: MozReview Request: Bug 1146295: Remove unused FuncZero function; r=h4writer

Comment 31

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: MozReview Request: Bug 1146295: Rename SimdTypeDescr::TYPE_SCALAR into SimdTypeDescr::TypeXn;