[webkit-reviews] review granted: [Bug 203563] [JSC] 32-bit platforms should use a PC base register : [Attachment 387844] Patch

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Caio Lima
<ticaiolima at gmail.com>'s request for review:
Bug 203563: [JSC] 32-bit platforms should use a PC base register
https://bugs.webkit.org/show_bug.cgi?id=203563

Attachment 387844: Patch

https://bugs.webkit.org/attachment.cgi?id=387844&action=review




--- Comment #12 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 387844
  --> https://bugs.webkit.org/attachment.cgi?id=387844
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387844&action=review

r=me with some comments.

> Source/JavaScriptCore/assembler/MIPSRegisters.h:53
> +    macro(r17, "s1",   0, 1) 		   \

Was this just a bug in the MIPS calling convention?

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:66
>  macro nextInstruction()
> -    loadb [PC], t0
> +    loadb [PB, PC, 1], t0
>      leap _g_opcodeMap, t1
>      jmp [t1, t0, 4], BytecodePtrTag
>  end
>  
>  macro nextInstructionWide16()
> -    loadb OpcodeIDNarrowSize[PC], t0
> +    loadb OpcodeIDNarrowSize[PB, PC, 1], t0
>      leap _g_opcodeMapWide16, t1
>      jmp [t1, t0, 4], BytecodePtrTag
>  end
>  
>  macro nextInstructionWide32()
> -    loadb OpcodeIDNarrowSize[PC], t0
> +    loadb OpcodeIDNarrowSize[PB, PC, 1], t0
>      leap _g_opcodeMapWide32, t1
>      jmp [t1, t0, 4], BytecodePtrTag
>  end
>  
>  macro getuOperandNarrow(opcodeStruct, fieldName, dst)
> -    loadb constexpr %opcodeStruct%_%fieldName%_index +
OpcodeIDNarrowSize[PC], dst
> +    loadb constexpr %opcodeStruct%_%fieldName%_index +
OpcodeIDNarrowSize[PB, PC, 1], dst
>  end
>  
>  macro getOperandNarrow(opcodeStruct, fieldName, dst)
> -    loadbsi constexpr %opcodeStruct%_%fieldName%_index +
OpcodeIDNarrowSize[PC], dst
> +    loadbsi constexpr %opcodeStruct%_%fieldName%_index +
OpcodeIDNarrowSize[PB, PC, 1], dst
>  end
>  
>  macro getuOperandWide16(opcodeStruct, fieldName, dst)
> -    loadh constexpr %opcodeStruct%_%fieldName%_index * 2 +
OpcodeIDWide16Size[PC], dst
> +    loadh constexpr %opcodeStruct%_%fieldName%_index * 2 +
OpcodeIDWide16Size[PB, PC, 1], dst
>  end
>  
>  macro getOperandWide16(opcodeStruct, fieldName, dst)
> -    loadhsi constexpr %opcodeStruct%_%fieldName%_index * 2 +
OpcodeIDWide16Size[PC], dst
> +    loadhsi constexpr %opcodeStruct%_%fieldName%_index * 2 +
OpcodeIDWide16Size[PB, PC, 1], dst
>  end
>  
>  macro getuOperandWide32(opcodeStruct, fieldName, dst)
> -    loadi constexpr %opcodeStruct%_%fieldName%_index * 4 +
OpcodeIDWide32Size[PC], dst
> +    loadi constexpr %opcodeStruct%_%fieldName%_index * 4 +
OpcodeIDWide32Size[PB, PC, 1], dst
>  end
>  
>  macro getOperandWide32(opcodeStruct, fieldName, dst)
> -    loadis constexpr %opcodeStruct%_%fieldName%_index * 4 +
OpcodeIDWide32Size[PC], dst
> +    loadis constexpr %opcodeStruct%_%fieldName%_index * 4 +
OpcodeIDWide32Size[PB, PC, 1], dst
>  end

Can you add a fix me to merge this with the ones in LowLevelInterpreter64.asm?
Seems like you can't right now because we don't have a loadhsp (although, I
think loadhsi is probably fine).