[webkit-reviews] review denied: [Bug 99154] Refactor MacroAssembler interfaces to differentiate the pointer operands from the 64-bit integer operands : [Attachment 168570] Rebased: Split patch - part 1: only add the *64 interfaces to MacroAssembler

[bugzilla-daemon at webkit.org]

Gavin Barraclough <barraclough at apple.com> has denied Yuqiang Xian
<yuqiang.xian at intel.com>'s request for 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 168570: Rebased: Split patch - part 1: only add the *64 interfaces
to MacroAssembler
https://bugs.webkit.org/attachment.cgi?id=168570&action=review

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=168570&action=review


Hi Yuquiang – this basically looks good, but per my previous comments please
move the *Ptr->*64 wrapper functions from MacroAssemblerX86_64.h to
MacroAssembler.h
That way, the Ptr->[32/64] mapping will be done in the same place for all
platforms.

> Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:862
> +    DataLabelPtr moveWithPatch(TrustedImm64 initialValue, RegisterID dest)

Hmmm, I need to think a little more about this – I think I'd be happy to land
as is & fix later as necessary.
The mismatch between TrustedImm64/DataLabelPtr means I think is probably isn't
correct.
We should probably either add DataLabel64, make DataLabelPtr map onto
DataLabel32 or DataLabel64 as appropriate, or keep the argument to this
function as TrustedImmPtr.
But we can probably land as is & revisit.