[Tarantool-patches] [PATCH luajit] Maintain chain invariant in DCE.
Sergey Bronnikov
sergeyb at tarantool.org
Mon Jun 17 11:44:29 MSK 2024
Hi, Sergey
thanks for update, LGTM.
On 16.06.2024 22:35, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> I've added comments and force-pushed the branch.
>
> On 14.06.24, Sergey Bronnikov wrote:
>> Hi, Sergey
>>
>> thanks for the patch! see my comment below
>>
>> On 17.05.2024 16:29, Sergey Kaplun wrote:
>>> From: Mike Pall <mike>
>>>
>>> Thanks to Peter Cawley.
>>>
>>> (cherry picked from commit f72c19e482b6f918b7cf42b0436e2b117d160a29)
>>>
>>> Instructions with strong guards that are sometimes emitted with a guard
>>> and sometimes emitted without a guard (like HREFK, CONV, or SLOAD) may
>>> be eliminated from the IR chain and replaced with the NOP IR. If the
>>> next IR of the same kind on the trace is not eliminated, it may
>>> reference the IR NOP instead of an instruction of the same type. This
>>> may lead to the corresponding assertion failure in the `rec_check_ir()`.
>>>
>>> This patch unconditionally links the IRs during chain maintenance in
>>> DCE.
>>>
>>> Sergey Kaplun:
>>> * added the description and the test for the problem
>>>
>>> Part of tarantool/tarantool#9924
>>> ---
>>>
> <snipped>
>
>>> +local counter = 0
>>> +-- luacheck: no unused
>>> +local tab = {}
>>> +while true do
>>> + -- The loop is still not recorded because the guard always
>>> + -- fails.
>>> + -- So, just try to compile it and check that there is no
>>> + -- assertion failure.
>>> + if counter > 2 then break end
>>> + counter = counter + 1
>>> + -- The pattern `-#{}` allows us to get CONV IRs. The first
>>> + -- appearance of this IR (in the `(-#{}).key`) is considered
>>> + -- unused by the compiler due to the corresponding "noop"
>>> + -- `__newindex` function.
>>> + -- The second usage of conversion (`tab[-#{}]`) is guarded (to
>>> + -- the int type), so it can't be eliminated.
>>> + -- As a result, the 0048 CONV references the 0039 NOP IR after
>>> + -- DCE in the IR chain.
>> I suppose an IR output would be helpful here. What do you think?
> I've added the corresponding output:
>
> | -- The IR itself looks like the following
> | -- by the `jit.dump` (NOPs are not printed):
> | -- 0036 num CONV 0035 num.int
> | -- 0037 num NEG 0036 0007
> | -- 0042 > tab TDUP {0x40154030}
> | -- 0043 int FLOAD 0014 tab.hmask
> | -- 0044 > int EQ 0043 +1
> | -- 0045 p32 FLOAD 0014 tab.node
> | -- 0046 > p32 HREFK 0045 "__newindex" @0
> | -- 0047 > fun HLOAD 0046
> | -- 0048 > fun EQ 0047 "lj-1094-ir-chain-dce.test.lua":20
> | -- 0049 > int CONV 0037 int.num index
>
> For some reason I reproduced this bug only for GC64 mode, so I've
> adjusted a IRs numbers and added the corresponding comment.
>
>
>>> + -- XXX: TDUP prevents the corresponding second usage from being
>>> + -- eliminated since the table insert semantics may change.
>>> + -- XXX: Use some numbers to simplify reading the `jit.dump`
>>> + -- output.
>>> + tab, tab[-#{}], (-#{}).key = {tdup = 'tdup'}, 1, 2
>>> +end
>>> +
>>> +test:ok(true, 'no assertion failure')
>>> +
>>> +test:done(true)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20240617/f9081862/attachment.htm>
More information about the Tarantool-patches
mailing list