| Summary: | B3 -O1 should not allocateStackByGraphColoring | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||||||||
| Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | buildbot, ggaren, jfbastien, keith_miller, mark.lam, msaboff, saam, ysuzuki | ||||||||||||||
| Priority: | P2 | ||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||
| Hardware: | All | ||||||||||||||||
| OS: | All | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Filip Pizlo
2017-04-11 12:06:55 PDT
Created attachment 306836 [details]
laid the foundation
Created attachment 306842 [details]
it is written
I haven't even compiled it yet.
Created attachment 306858 [details]
seems to work
Created attachment 306874 [details]
getting closer to done
Created attachment 306875 [details]
so close
Attachment 306875 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:36: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:463: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:463: Multi line control clauses should use braces. [whitespace/braces] [4]
Total errors found: 3 in 24 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 306877 [details]
the patch
Attachment 306877 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:36: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:463: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
ERROR: Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:463: Multi line control clauses should use braces. [whitespace/braces] [4]
Total errors found: 3 in 24 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 306877 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=306877&action=review r=me. > Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:307 > + [&] (Tmp, TmpData& data) -> bool { Why pass the Tmp if you don't use it anywhere? (In reply to Keith Miller from comment #9) > Comment on attachment 306877 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=306877&action=review > > r=me. > > > Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:307 > > + [&] (Tmp, TmpData& data) -> bool { > > Why pass the Tmp if you don't use it anywhere? Good point, I'll remove! Comment on attachment 306877 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=306877&action=review > Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:584 > + ptrdiff_t offset = -static_cast<ptrdiff_t>(m_code.frameSize()) - static_cast<ptrdiff_t>(entry.spillIndex) * 8 - 8; Why is the -8 needed? Also, is it worth using sizeof(void*) instead of 8? |