[webkit-reviews] review granted: [Bug 170161] Air should support linear scan for optLevel<2 : [Attachment 305896] the patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 170161: Air should support linear scan for optLevel<2
https://bugs.webkit.org/show_bug.cgi?id=170161

Attachment 305896: the patch

https://bugs.webkit.org/attachment.cgi?id=305896&action=review




--- Comment #21 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 305896
  --> https://bugs.webkit.org/attachment.cgi?id=305896
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=305896&action=review

r=me. Please fix the various places you have OOPS

> Source/JavaScriptCore/b3/air/AirAllocateRegistersByLinearScan.cpp:151
> +	       index += block->size() * 2;

shouldn't this be block->size()*2+1 since block->size()*2 is the tail?
Or is the head of one block OK to be the same as the tail?

> Source/JavaScriptCore/b3/air/AirAllocateRegistersByLinearScan.cpp:407
> +	       if (m_active.size() != m_registers.size()) {
> +		   bool didAssign = false;
> +		   for (Reg reg : m_registers) {
> +		       if (!m_activeRegs.contains(reg) &&
entry.possibleRegs.contains(reg)) {
> +			   assign(tmp, reg);
> +			   didAssign = true;
> +			   break;
> +		       }
> +		   }
> +		   if (didAssign)
> +		       continue;
> +	       }

We could totally do biased coloring here to try to remove some extra moves.
Should be cheap, and benefit quality of code in some programs. Code gen quality
isn't a priority for this phase, but if we get a throughput progression without
slowing this phase down, we should do it. Might be worth a FIXME. I'm happy to
hack it up.

> Source/JavaScriptCore/b3/air/AirAllocateRegistersByLinearScan.cpp:422
> +		   spill(tmp);

What guarantees !entry.isUnspillable here?