[webkit-reviews] review granted: [Bug 123195] Make JSCells have 32-bit Structure pointers : [Attachment 215667] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 123195: Make JSCells have 32-bit Structure pointers
https://bugs.webkit.org/show_bug.cgi?id=123195

Attachment 215667: Patch
https://bugs.webkit.org/attachment.cgi?id=215667&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=215667&action=review


> Source/JavaScriptCore/jit/JSInterfaceJIT.h:402
> -    ALWAYS_INLINE JSInterfaceJIT::Jump
JSInterfaceJIT::emitJumpIfNotType(RegisterID baseReg, RegisterID scratchReg,
JSType type)
> +    ALWAYS_INLINE void JSInterfaceJIT::emitLoadStructure(RegisterID source,
RegisterID dest, RegisterID scratch)
>      {
> -	   loadPtr(Address(baseReg, JSCell::structureOffset()), scratchReg);
> -	   return branch8(NotEqual, Address(scratchReg,
Structure::typeInfoTypeOffset()), TrustedImm32(type));
> +#if USE(JSVALUE64)
> +	   load32(MacroAssembler::Address(source, JSCell::structureIDOffset()),
dest);
> +	   loadPtr(vm()->heap.structureIDTable().base(), scratch); 
> +	   loadPtr(BaseIndex(scratch, dest, TimesEight), dest);
> +#else
> +	   UNUSED_PARAM(scratch);
> +	   loadPtr(MacroAssembler::Address(source,
JSCell::structureIDOffset()), dest);
> +#endif
> +    }

You could put this into AssemblyHelpers.

> Source/JavaScriptCore/jit/UnusedPointer.h:35
> +#if USE(JSVALUE64)
> +static const uintptr_t unusedPointer = 0x0;
> +#else
>  static const uintptr_t unusedPointer = 0xd1e7beef;
> +#endif

Kill if possible.

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:304
> +#	loadb Structure::m_indexingType[structure], scratch
> +#	storeb scratch, JSCell::m_indexingType[cell]
> +#
> +#	loadb Structure::m_typeInfo + TypeInfo::m_flags[structure], scratch
> +#	storeb scratch, JSCell::m_flags[cell]
> +#
> +#	loadb Structure::m_typeInfo + TypeInfo::m_type[structure], scratch
> +#	storeb scratch, JSCell::m_type[cell]
> +#
> +#	storep structure, JSCell::m_structureID[cell]

Delete.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:229
> +#	loadb Structure::m_indexingType[structure], scratch
> +#	storeb scratch, JSCell::m_indexingType[cell]
> +#
> +#	loadb Structure::m_typeInfo + TypeInfo::m_flags[structure], scratch
> +#	storeb scratch, JSCell::m_flags[cell]
> +#
> +#	loadb Structure::m_typeInfo + TypeInfo::m_type[structure], scratch
> +#	storeb scratch, JSCell::m_type[cell]
> +#
> +#	loadi Structure::m_structureID[structure], scratch
> +#	storei scratch, JSCell::m_structureID[cell]

Srsly bra