Bug 155270

Summary: DFG and FTL should constant-fold RegExpExec, RegExpTest, and StringReplace
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, cdumez, cmarcelo, commit-queue, ggaren, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
it begins
none
OMG this is so insane
none
I thnk that MaterializeNewObject is done
none
added RecordRegExpCachedResult
none
inching towards victory!
none
it is written
none
it compiles!
none
it does things!
none
more tests
none
passes all of the easy tests
none
maybe the patch
none
the patch
none
the patch
none
the patch
none
the patch
none
the patch
buildbot: commit-queue-
Archive of layout-test-results from ews114 for mac-yosemite
none
the patch
saam: review+
rebased patch
none
rebased patch
none
rebased patch
none
rebased patch none

Filip Pizlo
Reported 2016-03-09 15:40:00 PST
Patch forthcoming.
Attachments
it begins (33.79 KB, patch)
2016-03-09 15:40 PST, Filip Pizlo
no flags
OMG this is so insane (45.81 KB, patch)
2016-03-10 11:51 PST, Filip Pizlo
no flags
I thnk that MaterializeNewObject is done (58.09 KB, patch)
2016-03-10 12:51 PST, Filip Pizlo
no flags
added RecordRegExpCachedResult (71.38 KB, patch)
2016-03-10 14:12 PST, Filip Pizlo
no flags
inching towards victory! (80.70 KB, patch)
2016-03-10 15:03 PST, Filip Pizlo
no flags
it is written (85.08 KB, patch)
2016-03-10 15:35 PST, Filip Pizlo
no flags
it compiles! (87.15 KB, patch)
2016-03-10 18:54 PST, Filip Pizlo
no flags
it does things! (91.80 KB, patch)
2016-03-10 19:36 PST, Filip Pizlo
no flags
more tests (98.64 KB, patch)
2016-03-10 19:58 PST, Filip Pizlo
no flags
passes all of the easy tests (109.64 KB, patch)
2016-03-10 20:22 PST, Filip Pizlo
no flags
maybe the patch (111.92 KB, patch)
2016-03-10 20:52 PST, Filip Pizlo
no flags
the patch (112.40 KB, patch)
2016-03-10 21:09 PST, Filip Pizlo
no flags
the patch (112.79 KB, patch)
2016-03-10 21:30 PST, Filip Pizlo
no flags
the patch (126.52 KB, patch)
2016-03-10 23:10 PST, Filip Pizlo
no flags
the patch (126.53 KB, patch)
2016-03-10 23:58 PST, Filip Pizlo
no flags
the patch (126.80 KB, patch)
2016-03-11 00:15 PST, Filip Pizlo
buildbot: commit-queue-
Archive of layout-test-results from ews114 for mac-yosemite (1.10 MB, application/zip)
2016-03-11 01:09 PST, Build Bot
no flags
the patch (126.13 KB, patch)
2016-03-11 10:49 PST, Filip Pizlo
saam: review+
rebased patch (125.31 KB, patch)
2016-03-15 20:52 PDT, Filip Pizlo
no flags
rebased patch (125.35 KB, patch)
2016-03-19 18:23 PDT, Filip Pizlo
no flags
rebased patch (125.38 KB, patch)
2016-03-25 12:56 PDT, Filip Pizlo
no flags
rebased patch (125.41 KB, patch)
2016-04-04 21:33 PDT, Filip Pizlo
no flags
Filip Pizlo
Comment 1 2016-03-09 15:40:57 PST
Created attachment 273491 [details] it begins
Filip Pizlo
Comment 2 2016-03-09 15:45:43 PST
This creates a bunch of problems: - Yarr needs to be executed on the JIT thread. - DFG must model what happens in createRegExpMatchesArray, RegExpObject, and RegExpConstructor. - If the match succeeds, we must still emit code that creates the matches array, and that code should not be any less efficient than what we have in createRegExpMatchesArray. That code does a one-shot allocation that has enough room for the two names properties and enough room for the indexed properties. We currently have no DFG node that can do that. I'm attaching the Yarr concurrency problem just adding locks in various places. I will make the DFG duplicate the logic of createRegExpMatches array and friends. This will require creating new nodes, like a node for putting a result summary on the RegExp constructor. I'm attacking the array allocation problem by extending MaterializeNewObject to be able to do these kinds of one-shot allocations. Basically it will become the superset of NewArrayWithSize, NewArray, and NewObject. I'm also extending MaterializeNewObject to be compilable by the DFG tier, since we want to do this folding in DFG too.
Filip Pizlo
Comment 3 2016-03-10 11:51:15 PST
Created attachment 273590 [details] OMG this is so insane
Filip Pizlo
Comment 4 2016-03-10 12:51:44 PST
Created attachment 273602 [details] I thnk that MaterializeNewObject is done Now I can write the folding rule. This will be *SO MUCH FUN*.
Filip Pizlo
Comment 5 2016-03-10 14:12:20 PST
Created attachment 273621 [details] added RecordRegExpCachedResult
Filip Pizlo
Comment 6 2016-03-10 15:03:36 PST
Created attachment 273628 [details] inching towards victory!
Filip Pizlo
Comment 7 2016-03-10 15:35:42 PST
Created attachment 273638 [details] it is written I will eventually work up the courage to try to compile it.
Filip Pizlo
Comment 8 2016-03-10 18:54:43 PST
Created attachment 273665 [details] it compiles!
Filip Pizlo
Comment 9 2016-03-10 19:36:13 PST
Created attachment 273670 [details] it does things!
Filip Pizlo
Comment 10 2016-03-10 19:58:05 PST
Created attachment 273673 [details] more tests
Filip Pizlo
Comment 11 2016-03-10 20:22:31 PST
Created attachment 273675 [details] passes all of the easy tests
Filip Pizlo
Comment 12 2016-03-10 20:52:26 PST
Created attachment 273678 [details] maybe the patch
Filip Pizlo
Comment 13 2016-03-10 21:09:18 PST
Created attachment 273681 [details] the patch
WebKit Commit Bot
Comment 14 2016-03-10 21:12:05 PST
Attachment 273681 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:101: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSObject.h:101: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGInsertionSet.h:132: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1418: Declaration has space between type name and * in length * sizeof [whitespace/declaration] [3] Total errors found: 9 in 61 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 15 2016-03-10 21:30:48 PST
Created attachment 273684 [details] the patch Contains some more fixes.
WebKit Commit Bot
Comment 16 2016-03-10 21:33:39 PST
Attachment 273684 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:101: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSObject.h:101: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGInsertionSet.h:132: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1418: Declaration has space between type name and * in length * sizeof [whitespace/declaration] [3] Total errors found: 9 in 61 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 17 2016-03-10 23:10:08 PST
Created attachment 273691 [details] the patch
Filip Pizlo
Comment 18 2016-03-10 23:10:28 PST
Still running tests, not yet ready for review.
WebKit Commit Bot
Comment 19 2016-03-10 23:13:27 PST
Attachment 273691 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:101: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSObject.h:101: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGInsertionSet.h:132: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1418: Declaration has space between type name and * in length * sizeof [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/runtime/StringPrototype.h:66: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 10 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 20 2016-03-10 23:58:34 PST
Created attachment 273692 [details] the patch
WebKit Commit Bot
Comment 21 2016-03-11 00:02:53 PST
Attachment 273692 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:101: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSObject.h:101: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGInsertionSet.h:132: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1418: Declaration has space between type name and * in length * sizeof [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/runtime/StringPrototype.h:66: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 10 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 22 2016-03-11 00:15:07 PST
Created attachment 273695 [details] the patch with 32-bit fixes
WebKit Commit Bot
Comment 23 2016-03-11 00:16:12 PST
Attachment 273695 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:101: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSObject.h:101: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGInsertionSet.h:132: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1418: Declaration has space between type name and * in length * sizeof [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/runtime/StringPrototype.h:66: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 10 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 24 2016-03-11 01:08:59 PST
Comment on attachment 273695 [details] the patch Attachment 273695 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/959280 Number of test failures exceeded the failure limit.
Build Bot
Comment 25 2016-03-11 01:09:04 PST
Created attachment 273699 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Filip Pizlo
Comment 26 2016-03-11 10:48:05 PST
(In reply to comment #25) > Created attachment 273699 [details] > Archive of layout-test-results from ews114 for mac-yosemite > > The attached test failures were seen while running run-webkit-tests on the > mac-debug-ews. > Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5 LOL, this is because of a stale checkout situation on my end. I had added an assertion to RegExpMatchesArray.h as part of a previous abortive effort. That assertion was wrong. It ended up in my patch. And it set this bot on fire.
Filip Pizlo
Comment 27 2016-03-11 10:49:05 PST
Created attachment 273746 [details] the patch Reverts RegExpMatchesArray.h, in which I had accidentally added a wrong assertion that is unrelated to this patch. This should fix the debug tests.
WebKit Commit Bot
Comment 28 2016-03-11 10:50:35 PST
Attachment 273746 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:102: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSObject.h:102: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGInsertionSet.h:132: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1418: Declaration has space between type name and * in length * sizeof [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/runtime/StringPrototype.h:66: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 10 in 68 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 29 2016-03-11 14:07:12 PST
Comment on attachment 273746 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=273746&action=review r=me > Source/JavaScriptCore/ChangeLog:20 > + properties. Previously it could only handle objects (but not arrays) and naed properties typo: named > Source/JavaScriptCore/dfg/DFGLazyJSValue.cpp:120 > + dataLog("Giving up because of size limit.\n"); remove. > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:521 > + m_nodeIndex, origin, jsNumber(result.start), UntypedUse)); Why is this untyped use? > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:525 > + m_graph.m_varArgChildren.append(Edge(stringNode, UntypedUse)); Shouldn't this be StringUse? > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:651 > + int replLen = replace.length(); unsigned. > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:681 > + m_nodeIndex, origin, jsNumber(0), UntypedUse)); KnownInt32Use? > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6397 > + HashMap< > + int32_t, LValue, DefaultHash<int32_t>::Hash, > + WTF::UnsignedWithZeroKeyHashTraits<int32_t>> indexMap; style: I think this can be 1 line.
Filip Pizlo
Comment 30 2016-03-11 14:22:20 PST
(In reply to comment #29) > Comment on attachment 273746 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=273746&action=review > > r=me > > > Source/JavaScriptCore/ChangeLog:20 > > + properties. Previously it could only handle objects (but not arrays) and naed properties > > typo: named Fixed. > > > Source/JavaScriptCore/dfg/DFGLazyJSValue.cpp:120 > > + dataLog("Giving up because of size limit.\n"); > > remove. Fixed. > > > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:521 > > + m_nodeIndex, origin, jsNumber(result.start), UntypedUse)); > > Why is this untyped use? With the exception of the publicLength, vectorLength, and structure arguments to MaterializeNewObject, all of them are UntypedUse because they're just values you're storing into the object. You can store any JSValue into an object. Therefore, UntypedUse. More importantly, its not legal to just use whatever use kind you want anywhere you want. The signature of a DFG node is its NodeType and its UseKinds. If you use a UseKind that the node type doesn't recognize, you break the compiler. MaterializeNewObject does not recognize anything but UntypedUse for anything other than publicLength/vectorLength/structure. In general, if there is a use edge from A (the user) to B (the thing being used), then you don't pick the use kind based on what B is - you pick it based on what A wants B to be. If B is an int, you don't tell A to use it as an int unless A knows what to do with that information. > > > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:525 > > + m_graph.m_varArgChildren.append(Edge(stringNode, UntypedUse)); > > Shouldn't this be StringUse? See above. > > > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:651 > > + int replLen = replace.length(); > > unsigned. OK. I just copied what the StringPrototype code was doing. > > > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:681 > > + m_nodeIndex, origin, jsNumber(0), UntypedUse)); > > KnownInt32Use? See above, you can't just pick whatever use kind you like. > > > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6397 > > + HashMap< > > + int32_t, LValue, DefaultHash<int32_t>::Hash, > > + WTF::UnsignedWithZeroKeyHashTraits<int32_t>> indexMap; > > style: I think this can be 1 line. OK.
Saam Barati
Comment 31 2016-03-11 14:32:48 PST
(In reply to comment #30) > (In reply to comment #29) > > Comment on attachment 273746 [details] > > the patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=273746&action=review > > > > r=me > > > > > Source/JavaScriptCore/ChangeLog:20 > > > + properties. Previously it could only handle objects (but not arrays) and naed properties > > > > typo: named > > Fixed. > > > > > > Source/JavaScriptCore/dfg/DFGLazyJSValue.cpp:120 > > > + dataLog("Giving up because of size limit.\n"); > > > > remove. > > Fixed. > > > > > > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:521 > > > + m_nodeIndex, origin, jsNumber(result.start), UntypedUse)); > > > > Why is this untyped use? > > With the exception of the publicLength, vectorLength, and structure > arguments to MaterializeNewObject, all of them are UntypedUse because > they're just values you're storing into the object. You can store any > JSValue into an object. Therefore, UntypedUse. > > More importantly, its not legal to just use whatever use kind you want > anywhere you want. The signature of a DFG node is its NodeType and its > UseKinds. If you use a UseKind that the node type doesn't recognize, you > break the compiler. MaterializeNewObject does not recognize anything but > UntypedUse for anything other than publicLength/vectorLength/structure. > > In general, if there is a use edge from A (the user) to B (the thing being > used), then you don't pick the use kind based on what B is - you pick it > based on what A wants B to be. If B is an int, you don't tell A to use it > as an int unless A knows what to do with that information. > Makes sense. Thanks for clarifying. > > > > > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:525 > > > + m_graph.m_varArgChildren.append(Edge(stringNode, UntypedUse)); > > > > Shouldn't this be StringUse? > > See above. > > > > > > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:651 > > > + int replLen = replace.length(); > > > > unsigned. > > OK. I just copied what the StringPrototype code was doing. > > > > > > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:681 > > > + m_nodeIndex, origin, jsNumber(0), UntypedUse)); > > > > KnownInt32Use? > > See above, you can't just pick whatever use kind you like. > > > > > > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6397 > > > + HashMap< > > > + int32_t, LValue, DefaultHash<int32_t>::Hash, > > > + WTF::UnsignedWithZeroKeyHashTraits<int32_t>> indexMap; > > > > style: I think this can be 1 line. > > OK.
Filip Pizlo
Comment 32 2016-03-15 20:52:09 PDT
Created attachment 274169 [details] rebased patch
WebKit Commit Bot
Comment 33 2016-03-15 20:53:37 PDT
Attachment 274169 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:102: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSObject.h:102: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGInsertionSet.h:132: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1401: Declaration has space between type name and * in length * sizeof [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/runtime/StringPrototype.h:66: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 10 in 67 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 34 2016-03-19 18:23:11 PDT
Created attachment 274530 [details] rebased patch
WebKit Commit Bot
Comment 35 2016-03-19 18:24:43 PDT
Attachment 274530 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:102: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSObject.h:102: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGInsertionSet.h:132: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1414: Declaration has space between type name and * in length * sizeof [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/runtime/StringPrototype.h:66: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 10 in 67 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 36 2016-03-25 12:56:51 PDT
Created attachment 274929 [details] rebased patch
WebKit Commit Bot
Comment 37 2016-03-25 13:00:06 PDT
Attachment 274929 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:102: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSObject.h:102: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGInsertionSet.h:132: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1414: Declaration has space between type name and * in length * sizeof [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/runtime/StringPrototype.h:66: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 10 in 67 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 38 2016-04-04 21:33:54 PDT
Created attachment 275633 [details] rebased patch
WebKit Commit Bot
Comment 39 2016-04-04 21:36:57 PDT
Attachment 275633 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSObject.h:102: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSObject.h:102: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGInsertionSet.h:132: The parameter name "block" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:45: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1414: Declaration has space between type name and * in length * sizeof [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/runtime/StringPrototype.h:66: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 10 in 67 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 40 2016-04-05 15:13:41 PDT
Note You need to log in before you can comment on or make changes to this bug.