Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

[WIP]Copyreg fix. #8667

Closed
wants to merge 1 commit into from
Closed

[WIP]Copyreg fix. #8667

wants to merge 1 commit into from

Conversation

sivarv
Copy link
Member

@sivarv sivarv commented Dec 16, 2016

The following PR has caused an assert failure in SuperPMI playback
#8532

The repro method has the following IR pattern in one of the basic blocks:

// V1 is in esi at the beginning of the basic block
// Lower reg specification marks op1 as IsDelayFree=true and op2 is marked as contained immed.
// Further LOCKADD consumes its operands and produces no result.
GT_LOCKADD(op1=long static fied addr, op2 = 1)

// thisptr = rcx
GT_PUTARG_REG(v1)
ControlExpr = indir(lea(indir(lea(v1), 32)))
GT_CALL (thisptr, contrl expr)

Lower register specification marks op1 of GT_LOCKADD as isDelayFree=true.

--------------------------------+----+----+----+----+----+----+----+----+----+----+----+----+
Loc RP#  Name Type  Action Reg  |rax |rcx |rdx |rbx |rsp |rbp |rsi |rdi |r8  |r9  |r10 |r11 |
--------------------------------+----+----+----+----+----+----+----+----+----+----+----+----+
 91.#116 BB3 PredBB1            |    |    |    |----|----|----|V1 a|----|    |    |    |    |
 94.#117 C36  Def    Alloc rcx  |    |C36a|    |----|----|----|V1 a|----|    |    |    |    |
 97.#118 C36  Use *D Keep  rcx  |    |C36a|    |----|----|----|V1 a|----|    |    |    |    |
103.#119 rcx  Fixd   Keep  rcx  |    |C36a|    |----|----|----|V1 a|----|    |    |    |    |
103.#120 V1   Use    Copy  rcx  |    |V1 a|    |----|----|----|V1 a|----|    |    |    |    |
104.#121 rcx  Fixd   Keep  rcx  |    |V1 i|    |----|----|----|V1 i|----|    |    |    |    |
104.#122 I37  Def    Alloc rcx  |    |I37a|    |----|----|----|V1 i|----|    |    |    |    |

RefPos 117 and 118 correspond to Def and Use of op1 of GT_LOCKADD. After allocating 118, allocateRegisters() puts rcx in delayRegsToFree since it is the last Use. While processing FixedReg RefPos 119, allocateRegisters() assigns delayRegsToFree to regsToFree and sets the former to RBM_NONE.
While allocating RefPos 120, allocateRegister() finds that the location hasn't changed and hence will not free rcx (given by regsToFree local). Since V1 is rsi but needs to be in rcx, RefPos 120 is allocated rcx and marked as copyReg=true. Note that at the time of allocating rcx to RefPos 120, rcx is considered a busy reg.

Before processing RefPos 121, rcx gets freed by allocateRegisters() since the location has changed. As a result, V1 gets incorrectly marked as inactive.

@sivarv
Copy link
Member Author

sivarv commented Dec 17, 2016

@dotnet-bot test Windows_NT jitstressregs1
@dotnet-bot test Windows_NT jitstressregs2
@dotnet-bot test Windows_NT jitstressregs3
@dotnet-bot test Windows_NT jitstressregs4
@dotnet-bot test Windows_NT jitstressregs8
@dotnet-bot test Windows_NT jitstressregs0x10
@dotnet-bot test Windows_NT jitstressregs0x80
@dotnet-bot test Windows_NT jitstress2
@dotnet-bot test Windows_NT corefx_baseline
@dotnet-bot test Ubuntu jitstressregs1
@dotnet-bot test Ubuntu jitstressregs2
@dotnet-bot test Ubuntu jitstressregs3
@dotnet-bot test Ubuntu jitstressregs4
@dotnet-bot test Ubuntu jitstressregs8
@dotnet-bot test Ubuntu jitstressregs0x10
@dotnet-bot test Ubuntu jitstressregs0x80
@dotnet-bot test Ubuntu jitstress2
@dotnet-bot test Ubuntu corefx_baseline

@sivarv
Copy link
Member Author

sivarv commented Dec 17, 2016

@dotnet-bot test Windows_NT x86 Checked jitstress2_jitstressregs0x10
@dotnet-bot test Windows_NT x86 Checked jitstress2_jitstressregs0x80
@dotnet-bot test Windows_NT x86 Checked jitstress2_jitstressregs1
@dotnet-bot test Windows_NT x86 Checked jitstress2_jitstressregs2
@dotnet-bot test Windows_NT x86 Checked jitstress2_jitstressregs3
@dotnet-bot test Windows_NT x86 Checked jitstress2_jitstressregs4
@dotnet-bot test Windows_NT x86 Checked jitstress2_jitstressregs8
@dotnet-bot test Windows_NT x86 Checked jitstress2
@dotnet-bot test Windows_NT x86 Checked jitstressregs0x10
@dotnet-bot test Windows_NT x86 Checked jitstressregs0x80
@dotnet-bot test Windows_NT x86 Checked jitstressregs1
@dotnet-bot test Windows_NT x86 Checked jitstressregs2
@dotnet-bot test Windows_NT x86 Checked jitstressregs3
@dotnet-bot test Windows_NT x86 Checked jitstressregs4
@dotnet-bot test Windows_NT x86 Checked jitstressregs8

@sivarv sivarv closed this Dec 21, 2016
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants