Bug 157548 – Air may decide to put the result register of an arithmetic snippet in the tag register

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Saam Barati]

This causes our code for boxDouble to hilariously fail.

[Saam Barati]

Created attachment 278557 [details]
WIP

[Benjamin Poulain]

Comment on attachment 278557 [details]
WIP

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

> Source/JavaScriptCore/b3/B3LowerToAir.cpp:1008
> +            case ValueRep::LateUseRegister: // FIXME: correct?

Yep, that's fine.

> Source/JavaScriptCore/b3/B3LowerToAir.cpp:2194
> +                case ValueRep::LateUseRegister:

This should probably just be invalid.

> Source/JavaScriptCore/b3/B3Validate.cpp:337
> +                    //case ValueRep::LateUseRegister: // FIXME: is this validating result kind?

Yeah, you can probably make this rep invalid.

[Filip Pizlo]

Comment on attachment 278557 [details]
WIP

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

>> Source/JavaScriptCore/b3/B3LowerToAir.cpp:1008
>> +            case ValueRep::LateUseRegister: // FIXME: correct?
> 
> Yep, that's fine.

The other late thing is LateColdAny, which does not contain the word "Use".  So, I would call this ValueRep::LateRegister.

>> Source/JavaScriptCore/b3/B3Validate.cpp:337
>> +                    //case ValueRep::LateUseRegister: // FIXME: is this validating result kind?
> 
> Yeah, you can probably make this rep invalid.

+1

> Source/JavaScriptCore/b3/B3ValueRep.h:122
> +        return ValueRep(LateUse, reg);

Note, you could have written this as:

ValueRep result(reg);
result.m_kind = LateRegister;
return result;

And that would allow you to get rid of the LateUseTag overload of the constructor.  It's OK if you want to do it this way, though.

[Saam Barati]

Created attachment 278569 [details]
WIP

[Saam Barati]

*** Bug 157547 has been marked as a duplicate of this bug. ***

[Saam Barati]

Created attachment 278643 [details]
patch

[WebKit Commit Bot]

Attachment 278643 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/b3/testb3.cpp:11824:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:11842:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

*** Bug 157524 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 278643 [details]
patch

Clearing flags on attachment: 278643

Committed r200701: <http://trac.webkit.org/changeset/200701>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Geoffrey Garen]

No regression test for the snippets?

[Saam Barati]

(In reply to comment #11)
> No regression test for the snippets?

There is a test that has been failing for the last day that's already landed.