1127269 - More ARM cleanup

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P5)

Description

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Assembler-arm.h

This first patch fixes a number of issues I found by going through Assembler-arm.h:

* Made most single-arg constructors |explicit|
* A lot of classes now have private members
* Added some asserts when storing values in bitfields, to make sure we don't lose information
* A ton of style nits

I didn't make the Operand2(Address) constructor explicit yet, because that blows up. Will look into that separately.

Comment 1

[Douglas Crosher [:dougc]]

Comment on attachment 8556404 [details] [diff] [review]
Part 1 - Assembler-arm.h

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

Looks good, thank you.

::: js/src/jit/arm/Assembler-arm.h
@@ +729,5 @@
>      uint32_t data;
>  
>    protected:
>      DtrOff(datastore::Imm12Data immdata, IsUp_ iu)
> +      : data(immdata.encode() | uint32_t(IsImmDTR) | (uint32_t(iu)))

"(uint32_t(iu))" to "uint32_t(iu)" ?

Comment 2

[Nicolas B. Pierron [:nbp]]

The patch does not apply anymore.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Assembler-arm.h

Some of these changes were done in other bugs in the meantime. Here's a new patch for the remaining things.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Assembler-arm.h

Forgot to qref.

Comment 5

[Sean Stangl [:sstangl]]

Comment on attachment 8803868 [details] [diff] [review]
Part 1 - Assembler-arm.h

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

LGTM. Thank you for doing this.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/10205a882c3c
Clean up some code in the ARM backend. r=sstangl

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/10205a882c3c