[webkit-reviews] review granted: [Bug 72878] Strength reduction for Mul and Mod operations for known constants in DFG : [Attachment 116177] patch fixing the 64bit regression

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Yuqiang Xian
<yuqiang.xian at intel.com>'s request for review:
Bug 72878: Strength reduction for Mul and Mod operations for known constants in
DFG
https://bugs.webkit.org/show_bug.cgi?id=72878

Attachment 116177: patch fixing the 64bit regression
https://bugs.webkit.org/attachment.cgi?id=116177&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=116177&action=review


Looks great!  Two comments:

1) Can you check that the indentation is 4 spaces, no tabs, in the one place I
pointed out?

2) Since there is no clear win on major benchmarks, can you construct a micro
benchmark that demonstrates the win?  It can be arbitrarily simple.  I think
that this patch is a good idea, and it's the right direction, but we just need
to make sure that it does what we think it does.

I'm marking it r+ and cq- so that you can check the indentation.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1602
> +	   if (x) {
> +	     num = x;
> +	     log += shift;
> +	   }

Is the indentation right here?