[webkit-reviews] review denied: [Bug 99154] Refactor MacroAssembler interfaces to differentiate the pointer operands from the 64-bit integer operands : [Attachment 168383] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 12 17:40:52 PDT 2012
Gavin Barraclough <barraclough at apple.com> has denied review:
Bug 99154: Refactor MacroAssembler interfaces to differentiate the pointer
operands from the 64-bit integer operands
https://bugs.webkit.org/show_bug.cgi?id=99154
Attachment 168383: patch
https://bugs.webkit.org/attachment.cgi?id=168383&action=review
------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=168383&action=review
(r-, comments to follow)
> Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:278
> + explicit TrustedImm64(const void* value)
Do we really need this constructor? – might be nice to try to remove it – if
you have a void*, you should be creating a TrustedImmPtr.
> Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:286
> +
Extra newline.
> Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:299
> +
Extra newline.
> Source/JavaScriptCore/jit/JITOpcodes.cpp:1333
> + move(TrustedImm64(reinterpret_cast<uintptr_t>(globalThis)), regT0);
I think this code is implicitly making an assumption about JSValue boxing. I
think this would be better:
move(TrustedImm64(JSValue::encode(JSValue(globalThis))), regT0);
More information about the webkit-reviews
mailing list