1020122 - IonMonkey: (ARM) remove some unused arguments on internal assembler methods.

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Douglas Crosher [:dougc]]

The PoolEntry argument to as_Imm32Pool, as_FImm64Pool, and as_FImm32Pool is unused and always null, so can we removing it.

Comment 1

[Douglas Crosher [:dougc]]

Attachment: Remove some unused arguments on internal assembler methods.

Just a minor clean up.

Comment 2

[Marty Rosenberg [:mjrosenb]]

Comment on attachment 8433958 [details] [diff] [review]
Remove some unused arguments on internal assembler methods.

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

Interesting, I thought I explicitly added those because I needed them.  Oh well.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Do you have a Try link by chance? :)

Comment 4

[Douglas Crosher [:dougc]]

https://tbpl.mozilla.org/?tree=Try&rev=9dcb2e74b942

Comment 5

[Douglas Crosher [:dougc]]

Got a failure in the last try build. It does not appear to be related, but shall test again:
https://tbpl.mozilla.org/?tree=Try&rev=3e89cbe05fd3

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Instead of spinning a whole new round of builds and tests, you can just retrigger the one failed test job on the original push and save a lot of resources. I've gone ahead and canceled your second run and retriggered on the original one instead. Please read over the link below for further information. Thanks!
https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices

Comment 7

[Douglas Crosher [:dougc]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #6)
> Instead of spinning a whole new round of builds and tests, you can just
> retrigger the one failed test job on the original push and save a lot of
> resources. I've gone ahead and canceled your second run and retriggered on
> the original one instead. Please read over the link below for further
> information. Thanks!
> https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices

Thank you, I wish I could but I don't have required permission to retrigger or cancel jobs. This patch has been well tested locally.

Comment 8

[Douglas Crosher [:dougc]]

Try looks good, thank you.

Comment 9

[Douglas Crosher [:dougc]]

Comment on attachment 8433958 [details] [diff] [review]
Remove some unused arguments on internal assembler methods.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a, just a source clean up.

User impact if declined: None, but this small safe patch was split out from a larger pending fix and would block it.

Testing completed (on m-c, etc.): locally, and try build.

Risk to taking this patch (and alternatives if risky): very low.

String or IDL/UUID changes made by this patch: n/a

Comment 10

[Ryan VanderMeulen [:RyanVM]]

(In reply to Douglas Crosher [:dougc] from comment #7)
> Thank you, I wish I could but I don't have required permission to retrigger
> or cancel jobs. This patch has been well tested locally.

If you have the ability to push to Try, you should have access to self-serve. Please ping me on IRC and we can try to figure it out :)

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6df16ae5d1fb

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6df16ae5d1fb

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8433958 [details] [diff] [review]
Remove some unused arguments on internal assembler methods.

Taking because 31 is an ESR. Otherwise, I don't think I would have take it for beta.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/71e5514d3d84
https://hg.mozilla.org/releases/mozilla-beta/rev/a076e4f9d95d

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Kevin, is this something you think should be verified for beta? Anthony said I should probably check with you.

Comment 16

[Douglas Crosher [:dougc]]

(In reply to Liz Henry :lizzard from comment #15)
> Kevin, is this something you think should be verified for beta? Anthony said
> I should probably check with you.

It is a very safe patch. If it compiles it is ok.

Further it is being fuzz tested as part of bug 1026919, and everything looks good so far.

If you are considering investing in some verification for beta then could you consider jumping directly to the combined patch in bug 1026919.

Comment 17

[Kevin Brosnan [Ex-Mozilla]]

No. Fuzz testing is the correct method here.