149713 2015-10-01 09:51:49 -0700 DFG peephole does not have effect on CompareEq / CompareStrictEq because of MovHint 2018-03-22 05:02:23 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED DUPLICATE 177100 https://bugs.webkit.org/show_bug.cgi?id=149616 P2 Normal --- 1 ysuzuki webkit-unassigned fpizlo ggaren keith_miller mark.lam msaboff saam oldest_to_newest 1129952 0 ysuzuki 2015-10-01 09:51:49 -0700 Currently, we have op_jless, op_jgreater etc. But we don't have op_jeq. Instead we compile the following function eq(a, b) { if (a === b) return true; return false; } to eq#CU1c7h:[0x7fb57e6dd268->0x7fb57e46c200, NoneFunctionCall, 17]: 17 m_instructions; 136 bytes; 3 parameter(s); 8 callee register(s); 6 variable(s) [ 0] enter [ 1] get_scope loc3 [ 3] mov loc4, loc3 [ 6] eq loc6, arg1, arg2 [ 10] jfalse loc6, 5(->15) [ 13] ret True(const1) [ 15] ret False(const2) So, in DFG, between op_eq and op_jfalse, we have MovHint to store the result of op_eq. As a result, detectPeepHoleBranch() never detects peep hole for that case. 1129962 1 fpizlo 2015-10-01 10:06:57 -0700 When and if we implement a fix that allows the peephole detection to "skip" the MovHint, we should check how much of a speed-up it is. If it is not a speed-up, then instead of writing a patch that extends the peephole to skip MovHint, we should write a patch that removes all code for those peepholes that are currently dead because of the MovHint. That peephole code is a pain to maintain and it's possible that it buys us nothing. 1129967 2 ysuzuki 2015-10-01 10:23:49 -0700 (In reply to comment #1) > When and if we implement a fix that allows the peephole detection to "skip" > the MovHint, we should check how much of a speed-up it is. > > If it is not a speed-up, then instead of writing a patch that extends the > peephole to skip MovHint, we should write a patch that removes all code for > those peepholes that are currently dead because of the MovHint. That > peephole code is a pain to maintain and it's possible that it buys us > nothing. Right. This peep hole optimization is easily broken because the assumption the optimization rely on is fragile. If it does not provide significant performance improvement, dropping these code is better. 1130012 3 saam 2015-10-01 12:58:35 -0700 Yeah, I remember running into not being able to get peephole optimization to kick in because of MovHints (that's why I asked you to upload a test for that, because I thought you found a way for it to kick in). 1408328 4 ysuzuki 2018-03-22 05:02:23 -0700 *** This bug has been marked as a duplicate of bug 177100 ***