Bug 156343 – [Win] Fix for JSC stress test failures.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

We need to make it clear to MSVC that the method loadPtr(ImplicitAddress address, RegisterID dest) should be used, and not loadPtr(const void* address, RegisterID dest).

Created attachment 275891 [details]
Patch

[Alex Christensen]

Comment on attachment 275891 [details]
Patch

This seems to be used a few other places, but I don't understand how a GPRInfo is being interpreted as a void*.

[Filip Pizlo]

Comment on attachment 275891 [details]
Patch

Wow, crazy!

Thanks for reviewing :)

Committed r199160: <http://trac.webkit.org/changeset/199160>

[Filip Pizlo]

By the way: the fact that load and friends take const void* instead of AbsoluteAddress and they take GPRReg instead of Address (via the weird ImplicitAddress thing) is a vestige of a past long gone.

It would be super cool to replace all cases of the assembler taking void* and change them to take AbsoluteAddress.  It would also be cool to get rid of ImplicitAddress and make everyone use Address.

It's not obvious why MSVC interpreted the code the way that it did, but I can sort of see it, and it's scary to think just how much our load() and store() methods rely on very specific interpretations of overload resolution to work properly.  A RegisterID/GPRReg is just an enum, which coerces to int, and regT0 is a zero-valued enum.  We want MSVC to realize that the preferred overload resolution is to coerce to ImplicitAddress, but apparently due to the zeroish nature of regT0, it's treating it as null-like and coercing to void*.  Whether that's right or wrong, I don't really care, because I don't like it when the correctness of code relies so much on C++'s arcane overload resolution rules.  Therefore, I think that this change is a big win for correctness, and I think we should use Address() explicitly in more places.