WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
157548
Air may decide to put the result register of an arithmetic snippet in the tag register
https://bugs.webkit.org/show_bug.cgi?id=157548
Summary
Air may decide to put the result register of an arithmetic snippet in the tag...
Saam Barati
Reported
2016-05-10 17:44:35 PDT
This causes our code for boxDouble to hilariously fail.
Attachments
WIP
(14.65 KB, patch)
2016-05-10 17:44 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
WIP
(14.00 KB, patch)
2016-05-10 19:08 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
patch
(17.23 KB, patch)
2016-05-11 11:50 PDT
,
Saam Barati
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Saam Barati
Comment 1
2016-05-10 17:44:57 PDT
Created
attachment 278557
[details]
WIP
Benjamin Poulain
Comment 2
2016-05-10 18:02:19 PDT
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 3
2016-05-10 18:38:50 PDT
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
Comment 4
2016-05-10 19:08:19 PDT
Created
attachment 278569
[details]
WIP
Saam Barati
Comment 5
2016-05-11 00:13:53 PDT
***
Bug 157547
has been marked as a duplicate of this bug. ***
Saam Barati
Comment 6
2016-05-11 11:50:14 PDT
Created
attachment 278643
[details]
patch
WebKit Commit Bot
Comment 7
2016-05-11 11:53:12 PDT
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
Comment 8
2016-05-11 12:14:02 PDT
***
Bug 157524
has been marked as a duplicate of this bug. ***
WebKit Commit Bot
Comment 9
2016-05-11 13:53:20 PDT
Comment on
attachment 278643
[details]
patch Clearing flags on attachment: 278643 Committed
r200701
: <
http://trac.webkit.org/changeset/200701
>
WebKit Commit Bot
Comment 10
2016-05-11 13:53:24 PDT
All reviewed patches have been landed. Closing bug.
Geoffrey Garen
Comment 11
2016-05-11 14:30:02 PDT
No regression test for the snippets?
Saam Barati
Comment 12
2016-05-11 14:52:15 PDT
(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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug