1293606 - IonMonkey: MIPS: Import Loongson optimizations to ma_load and ma_store baseindex

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[hev [:hev]]

1. MacroAssemblerMIPSShared::ma_load(Register dest, const BaseIndex& src, LoadStoreSize size, LoadStoreExtension extension);
2. MacroAssemblerMIPSShared::ma_store(Register data, const BaseIndex& dest, LoadStoreSize size, LoadStoreExtension extension);
3. MacroAssemblerMIPSShared::ma_store(Imm32 imm, const BaseIndex& dest, LoadStoreSize size, LoadStoreExtension extension);

Comment 1

[hev [:hev]]

Attachment: 0001-Bug-1293606-IonMonkey-MIPS-Import-Loongson-optimizat.patch

Comment 2

[Tooru Fujisawa [:arai]]

out of curiosity, what does "gs" means in "as_gslbx" and "InstGS" etc?

Comment 3

[hev [:hev]]

(In reply to Tooru Fujisawa [:arai] from comment #2)
> out of curiosity, what does "gs" means in "as_gslbx" and "InstGS" etc?

Haha, "gs" means "godson", is Loongson's old name, and is a usual prefix for Loongson-specific instructions :)

https://sourceware.org/git/?p=binutils.git;a=blob;f=opcodes/mips-opc.c;h=4f729313ed5352b58b32f44d2800a4dd1bd30e57;hb=HEAD#l441

Comment 4

[Tooru Fujisawa [:arai]]

(In reply to Heiher [:hev] from comment #3)
> (In reply to Tooru Fujisawa [:arai] from comment #2)
> > out of curiosity, what does "gs" means in "as_gslbx" and "InstGS" etc?
> 
> Haha, "gs" means "godson", is Loongson's old name, and is a usual prefix for
> Loongson-specific instructions :)
> 
> https://sourceware.org/git/?p=binutils.git;a=blob;f=opcodes/mips-opc.c;
> h=4f729313ed5352b58b32f44d2800a4dd1bd30e57;hb=HEAD#l441

Is there any spec for those instruction?
I cannot find them in specs I have.

Comment 5

[Tooru Fujisawa [:arai]]

Comment on attachment 8779282 [details] [diff] [review]
0001-Bug-1293606-IonMonkey-MIPS-Import-Loongson-optimizat.patch

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

for now, reviewed other parts than switches for as_gs*

::: js/src/jit/mips-shared/MacroAssembler-mips-shared.cpp
@@ +384,5 @@
>                                    LoadStoreSize size, LoadStoreExtension extension)
>  {
> +    if (isLoongson() && ZeroExtend != extension &&
> +        Imm8::IsInSignedRange(src.offset))
> +    {

above 3 lines should fit into 1 line (99 chars)

@@ +389,5 @@
> +        Register index = src.index;
> +
> +        if (src.scale != TimesOne) {
> +            int32_t shift = Imm32::ShiftOf(src.scale).value;
> +            index = SecondScratchReg;

it would be nice to assert scale.base is not SecondScratchReg.
I tried to follow callsites to make sure it's not, but there are too much.

@@ +425,5 @@
>  MacroAssemblerMIPSShared::ma_store(Register data, const BaseIndex& dest,
>                                     LoadStoreSize size, LoadStoreExtension extension)
>  {
> +    if (isLoongson() && Imm8::IsInSignedRange(dest.offset))
> +    {

please put "{" at the end of "if (...)" line

@@ +430,5 @@
> +        Register index = dest.index;
> +
> +        if (dest.scale != TimesOne) {
> +            int32_t shift = Imm32::ShiftOf(dest.scale).value;
> +            index = SecondScratchReg;

here too, please assert dest.base is not SecondScratchReg

@@ +466,5 @@
>  MacroAssemblerMIPSShared::ma_store(Imm32 imm, const BaseIndex& dest,
>                                     LoadStoreSize size, LoadStoreExtension extension)
>  {
> +    if (isLoongson() && Imm8::IsInSignedRange(dest.offset))
> +    {

please put "{" at the end of "if (...)" line

@@ +472,5 @@
> +        Register index = dest.index;
> +
> +        if (imm.value) {
> +            data = ScratchRegister;
> +            ma_li(data, imm);

please assert dest.base and dest.index are not ScratchRegister.

@@ +477,5 @@
> +        }
> +
> +        if (dest.scale != TimesOne) {
> +            int32_t shift = Imm32::ShiftOf(dest.scale).value;
> +            index = SecondScratchReg;

and here too, please assert dest.base is not SecondScratchReg.

Comment 6

[hev [:hev]]

(In reply to Tooru Fujisawa [:arai] from comment #4)
> (In reply to Heiher [:hev] from comment #3)
> > (In reply to Tooru Fujisawa [:arai] from comment #2)
> > > out of curiosity, what does "gs" means in "as_gslbx" and "InstGS" etc?
> > 
> > Haha, "gs" means "godson", is Loongson's old name, and is a usual prefix for
> > Loongson-specific instructions :)
> > 
> > https://sourceware.org/git/?p=binutils.git;a=blob;f=opcodes/mips-opc.c;
> > h=4f729313ed5352b58b32f44d2800a4dd1bd30e57;hb=HEAD#l441
> 
> Is there any spec for those instruction?
> I cannot find them in specs I have.

==== gslbx - Load Byte Indexed ====

31      26 25      21 20     16 15       11 10       3 2     0
LDC2      |   base   |    rt   |   index   |  offset  |  BYTE
110110                                                   000

Format: GSLBX	rt, offset(base, index)
To load a byte from memory as a signed value (GPR + GPR addressing)

Description: GPR[rt] <--- memory[GPR[base] + GPR[index] + offset]
The contents of the 8-bit byte at the memory location specified by the effective address are fetched, sign-extend, and placed in GPR rt. The 8-bit signed offset is added to the contents of GPR base + GPR index to form the effective address.

Restrictions:
None

Exceptions:
TLB Refill, TLB Invalid, Address Error, Watch


==== gslhx - Load Halfword Indexed ====

31      26 25      21 20     16 15       11 10       3 2     0
LDC2      |   base   |    rt   |   index   |  offset  |  HALF
110110                                                   001

Format: GSLHX	rt, offset(base, index)
To load a halfword from memory as a signed value (GPR + GPR addressing)

Description: GPR[rt] <--- memory[GPR[base] + GPR[index] + offset]
The contents of the 16-bit byte at the memory location specified by the aligned effective address are fetched, sign-extend, and placed in GPR rt. The 8-bit signed offset is added to the contents of GPR base + GPR index to form the effective address.

Restrictions:
The effective address must be naturally-aligned. If the least-significant bit of the address is non-zero, an Address Error exception occurs.

Exceptions:
TLB Refill, TLB Invalid, Address Error, Watch


==== gslwx - Load Word Indexed ====

31      26 25      21 20     16 15       11 10       3 2     0
LDC2      |   base   |    rt   |   index   |  offset  |  WORD
110110                                                   010

Format: GSLWX	rt, offset(base, index)
To load a word from memory as a signed value (GPR + GPR addressing)

Description: GPR[rt] <--- memory[GPR[base] + GPR[index] + offset]
The contents of the 32-bit byte at the memory location specified by the aligned effective address are fetched, sign-extend, and placed in GPR rt. The 8-bit signed offset is added to the contents of GPR base + GPR index to form the effective address.

Restrictions:
The effective address must be naturally-aligned. If either of the 2 least-significant bit of the address is non-zero, an Address Error exception occurs.

Exceptions:
TLB Refill, TLB Invalid, Address Error, Watch


==== gsldx - Load Doubleword Indexed ====

31      26 25      21 20     16 15       11 10       3 2     0
LDC2      |   base   |    rt   |   index   |  offset  |  DWORD
110110                                                   011

Format: GSLWX	rt, offset(base, index)
To load a doubleword from memory (GPR + GPR addressing)

Description: GPR[rt] <--- memory[GPR[base] + GPR[index] + offset]
The contents of the 64-bit byte at the memory location specified by the aligned effective address are fetched, sign-extend, and placed in GPR rt. The 8-bit signed offset is added to the contents of GPR base + GPR index to form the effective address.

Restrictions:
The effective address must be naturally-aligned. If any of the 3 least-significant bit of the address is non-zero, an Address Error exception occurs.

Exceptions:
TLB Refill, TLB Invalid, Address Error, Watch


==== gssbx - Store Byte Indexed ====

31      26 25      21 20     16 15       11 10       3 2     0
SDC2      |   base   |    rt   |   index   |  offset  |  BYTE
111110                                                   000

Format: GSSBX	rt, offset(base, index)
To store a byte to memory (GPR + GPR addressing)

Description: memory[GPR[base] + GPR[index] + offset] <--- GPR[rt]
The least-significant 8-bit of GPR rt is stored in memory at the location specified by the effective address. The 8-bit signed offset is added to the contents of GPR base + GPR index to form the effective address.

Restrictions:
None

Exceptions:
TLB Refill, TLB Invalid, TLB Modified, Address Error, Watch


==== gsshx - Store Halfword Indexed ====

31      26 25      21 20     16 15       11 10       3 2     0
SDC2      |   base   |    rt   |   index   |  offset  |  HALF
111110                                                   001

Format: GSSHX	rt, offset(base, index)
To store a halfword to memory (GPR + GPR addressing)

Description: memory[GPR[base] + GPR[index] + offset] <--- GPR[rt]
The least-significant 16-bit of GPR rt is stored in memory at the location specified by the aligned effective address. The 8-bit signed offset is added to the contents of GPR base + GPR index to form the effective address.

Restrictions:
The effective address must be naturally-aligned. If the least-significant bit of the address is non-zero, an Address Error exception occurs.

Exceptions:
TLB Refill, TLB Invalid, TLB Modified, Address Error, Watch


==== gsswx - Store Word Indexed ====

31      26 25      21 20     16 15       11 10       3 2     0
SDC2      |   base   |    rt   |   index   |  offset  |  WORD
111110                                                   010

Format: GSSHX	rt, offset(base, index)
To store a word to memory (GPR + GPR addressing)

Description: memory[GPR[base] + GPR[index] + offset] <--- GPR[rt]
The least-significant 32-bit of GPR rt is stored in memory at the location specified by the aligned effective address. The 8-bit signed offset is added to the contents of GPR base + GPR index to form the effective address.

Restrictions:
The effective address must be naturally-aligned. If either of the 2 least-significant bit of the address is non-zero, an Address Error exception occurs.

Exceptions:
TLB Refill, TLB Invalid, TLB Modified, Address Error, Watch


==== gssdx - Store Doubleword Indexed ====

31      26 25      21 20     16 15       11 10       3 2     0
SDC2      |   base   |    rt   |   index   |  offset  |  DWORD
111110                                                   011

Format: GSSHX	rt, offset(base, index)
To store a doubleword to memory (GPR + GPR addressing)

Description: memory[GPR[base] + GPR[index] + offset] <--- GPR[rt]
The least-significant 64-bit of GPR rt is stored in memory at the location specified by the aligned effective address. The 8-bit signed offset is added to the contents of GPR base + GPR index to form the effective address.

Restrictions:
The effective address must be naturally-aligned. If any of the 3 least-significant bit of the address is non-zero, an Address Error exception occurs.

Exceptions:
TLB Refill, TLB Invalid, TLB Modified, Address Error, Watch

Comment 7

[hev [:hev]]

Attachment: 0001-Bug-1293606-IonMonkey-MIPS-Import-Loongson-optimizat.patch

Thank you.

Comment 8

[Tooru Fujisawa [:arai]]

Comment on attachment 8779585 [details] [diff] [review]
0001-Bug-1293606-IonMonkey-MIPS-Import-Loongson-optimizat.patch

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

Thank you for the spec and the patch :)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b52c3c82732
IonMonkey: MIPS: Import Loongson optimizations to ma_load and ma_store baseindex. r=arai

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7b52c3c82732