[webkit-reviews] review denied: [Bug 128626] Lowering of CheckArray in FTL not supported for all types. : [Attachment 223905] Added more functionality to CheckArray

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has denied Matthew Mirman <mmirman at apple.com>'s
request for review:
Bug 128626: Lowering of CheckArray in FTL not supported for all types.
https://bugs.webkit.org/show_bug.cgi?id=128626

Attachment 223905: Added more functionality to CheckArray
https://bugs.webkit.org/attachment.cgi?id=223905&action=review

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


> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:-1756
> -	   LValue cell = lowCell(edge);

Why remove this here?

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1776
> +	       LValue cell = lowCell(edge);	       

And move it here?

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1792
> +	   LValue cell = lowCell(m_node->child1());

... and then have to do it again here?	Seems like it was happiest at the top.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1794
> +	   LValue obj = m_out.loadPtr(m_out.address(cell ,
m_heaps.JSCell_structure)); 

I wouldn't call this "obj".  I would call it "structure".

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1798
> +		   , m_out.constIntPtr(expectedClassInfo)));

Weird indentation and comma placement.	Put the comma at the end of the
previous line.