[webkit-changes] [WebKit/WebKit] 6f477c: VectorBitwiseSelect should not create tricky IR to...

Justin Michaud noreply at github.com
Fri Feb 17 09:41:40 PST 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 6f477c95e4740dcab092e7320f8d0fbd681bad82
      https://github.com/WebKit/WebKit/commit/6f477c95e4740dcab092e7320f8d0fbd681bad82
  Author: Justin Michaud <justin_michaud at apple.com>
  Date:   2023-02-17 (Fri, 17 Feb 2023)

  Changed paths:
    A JSTests/wasm/stress/simd-regalloc-stress.js
    M Source/JavaScriptCore/b3/B3LowerToAir.cpp
    M Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp
    M Source/JavaScriptCore/b3/air/AirInst.cpp

  Log Message:
  -----------
  VectorBitwiseSelect should not create tricky IR to colour
https://bugs.webkit.org/show_bug.cgi?id=252424
rdar://105390761

Reviewed by Yusuke Suzuki.

This test stresses the UD register on Arm64 for bitselect.

Air     MoveVector (%tmp0), %ftmp12
Air     MoveVector 16(%tmp0), %ftmp13
Air     MoveVector 32(%tmp0), %ftmp11
Air     MoveVector %ftmp11, %ftmp10
Air     VectorBitwiseSelect %ftmp12, %ftmp13, %ftmp10
Air     MoveVector %ftmp10, %*ftmp8*
(ftmp11, ftmp10 are unspillable here because it is Use'd right after
being Def'd, and the range is size 1)
...
Air     Patch &CCall1, %tmp1, %x0, %x1, %q0, %x0, %x1
Air     MoveDoubleConditionallyTest32 NonZero, %tmp4, %tmp4, %*ftmp8*, %ftmp9, %ftmp6

Then, after this iteration, we coalesce ftmp11 -> ftmp10 -> ftmp8:

Air     MoveVector 32(%x23), %ftmp11
Air     VectorBitwiseSelect %ftmp12, %ftmp13, %ftmp11

But ftmp11 is still unspillable, and now the graph is uncolourable.

Instead, we should emit this:

Air     MoveVector %ftmp8, %ftmp6
Air     VectorBitwiseSelect %ftmp9, %ftmp10, %ftmp6
...
Air     MoveDoubleConditionallyTest32 NonZero, %x21, %x21, %ftmp6, %ftmp7, %ftmp4

Here, the register allocator can see the lifetime of ftmp6 right away, so
we always consider spilling it.

In general, should the register allocator handle this case gracefully?
Probably. We should either:
1) Not coalesce unspillable registers until we know the graph is colourable
2) Remove registers from unspillableTmps when we increase their live range

There is a decent chance that these changes could break things, or be
subtly wrong in a different case. Let's just emit code that is easy to
colour.

* Source/JavaScriptCore/b3/B3LowerToAir.cpp:
* Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:
* Source/JavaScriptCore/b3/air/AirInst.cpp:
(JSC::B3::Air::Inst::dump const):

Canonical link: https://commits.webkit.org/260449@main




More information about the webkit-changes mailing list