Bug 151474 – [JSC] Add Air lowering to BitNot() for Xor(value, -1)

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[JSC] Add B3 BitNot and Strength Reduction

[Benjamin Poulain]

Created attachment 265927 [details]
Patch

[Filip Pizlo]

Comment on attachment 265927 [details]
Patch

We don't need a new opcode for this. BitNot(x) is just BitXor(x, -1). I don't think it's useful to have a BitNot instruction when BitXor with an immediate serves the same purpose. Also, it would be weird to have BitNot but not Neg - Neg is currently handled using Sub(0, x), sort of like how I'm proposing to handle Not using Xor.

[Benjamin Poulain]

(In reply to comment #2)
> Comment on attachment 265927 [details]
> Patch
> 
> We don't need a new opcode for this. BitNot(x) is just BitXor(x, -1). I
> don't think it's useful to have a BitNot instruction when BitXor with an
> immediate serves the same purpose. Also, it would be weird to have BitNot
> but not Neg - Neg is currently handled using Sub(0, x), sort of like how I'm
> proposing to handle Not using Xor.

Yep, that's a better idea.

[Benjamin Poulain]

Created attachment 265934 [details]
Patch

[Filip Pizlo]

Comment on attachment 265934 [details]
Patch

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

> Source/JavaScriptCore/b3/B3LowerToAir.cpp:1319
> +                appendUnOp<Not32, Not64, Air::Oops>(m_value->child(0));

this is missing a break statement. ;-)

[Benjamin Poulain]

Created attachment 265979 [details]
Patch

[Filip Pizlo]

Comment on attachment 265979 [details]
Patch

Don't forget the Store(Not(Load)) case.

[Benjamin Poulain]

Created attachment 265981 [details]
Patch

[Benjamin Poulain]

Comment on attachment 265981 [details]
Patch

Clearing flags on attachment: 265981

Committed r192696: <http://trac.webkit.org/changeset/192696>

[Benjamin Poulain]

All reviewed patches have been landed.  Closing bug.