Bug 150919 – [JSC] Add B3-to-Air lowering for the shift opcodes

• Component: New Bugs
• Status: RESOLVED FIXED

[Benjamin Poulain]

[JSC] Add B3-to-Air lowering for the shift opcodes

[Benjamin Poulain]

Created attachment 264835 [details]
Patch

[Filip Pizlo]

Comment on attachment 264835 [details]
Patch

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

> Source/JavaScriptCore/b3/B3Const32Value.cpp:105
> +Value* Const32Value::shlConstant(Procedure& proc, Value* other) const
> +{
> +    if (!other->hasInt32())
> +        return nullptr;
> +    return proc.add<Const32Value>(origin(), m_value << other->asInt32());
> +}
> +
> +Value* Const32Value::sShrConstant(Procedure& proc, Value* other) const
> +{
> +    if (!other->hasInt32())
> +        return nullptr;
> +    return proc.add<Const32Value>(origin(), m_value >> other->asInt32());
> +}
> +
> +Value* Const32Value::zShrConstant(Procedure& proc, Value* other) const
> +{
> +    if (!other->hasInt32())
> +        return nullptr;
> +    return proc.add<Const32Value>(origin(), static_cast<int32_t>(static_cast<uint32_t>(m_value) >> other->asInt32()));
> +}
> +

You should explicitly mask the shift amount.

I propose that we define shifts in B3 to be defined so that:

    Shl(a, b)

where a is In32 is:

    a << (b & 31)

and where a is Int64 it's:

    a << (b & 63)

The reason why this is important is that this matches exactly what X86_64 and ARM do, and it's what FTL lowering will expect.

But in C, the upper bits of the shit amount have an undefined effect on the outcome of the shift.

Therefore, when using C code to fold shifts, we need to mask.

And of course I had to file: https://bugs.webkit.org/show_bug.cgi?id=150924

[Benjamin Poulain]

Committed r192051: <http://trac.webkit.org/changeset/192051>