diff options
author | sivarv <sivarv@microsoft.com> | 2016-01-12 13:37:07 -0800 |
---|---|---|
committer | sivarv <sivarv@microsoft.com> | 2016-01-12 13:37:07 -0800 |
commit | ef43adba2c61b68f601cfce5ac6b3937ce5ded21 (patch) | |
tree | 4db65ac27a3db49b1f3d6643329feffeec59a391 /src | |
parent | db743bc4103e78484bd0dac54a1b887da63a7059 (diff) | |
download | coreclr-ef43adba2c61b68f601cfce5ac6b3937ce5ded21.tar.gz coreclr-ef43adba2c61b68f601cfce5ac6b3937ce5ded21.tar.bz2 coreclr-ef43adba2c61b68f601cfce5ac6b3937ce5ded21.zip |
Fix to github issue #2349.
For the repro case the method being compiled is a generic method
whose generic context is derived from thisptr. For this reason JIT
is asked to keep thisptr alive.
Due to tail call loop optimization, the recursive method call at the end
is converted into a loop. This transformation assigns all the arguments
to temps and temps back to incoming parameters of the method and finally
branches to the first basic block. This will leads to the following basic
block
tmp = GT_NODE(thisptr)
thisptr = tmp
Here we have both use and def of thisptr. Lsra.cpp::SetLastUses() has
a bug in not properly accounting for thiptr, that leads to an assert failure.
Also, computeUpdateLifeVar()also needs be fixed to not consider
thisptr being born due to assignment of temp to thisptr.
Diffstat (limited to 'src')
-rw-r--r-- | src/jit/codegencommon.cpp | 4 | ||||
-rw-r--r-- | src/jit/lsra.cpp | 4 |
2 files changed, 7 insertions, 1 deletions
diff --git a/src/jit/codegencommon.cpp b/src/jit/codegencommon.cpp index 40dec4a791..49c00622b3 100644 --- a/src/jit/codegencommon.cpp +++ b/src/jit/codegencommon.cpp @@ -891,6 +891,10 @@ void Compiler::compUpdateLifeVar(GenTreePtr tree, VARSET_TP* pLastUseVars) if (!varDsc->lvTracked && !varDsc->lvPromoted) return; + // Don't update life for thisptr that is kept alive + if (lvaKeepAliveAndReportThis() && lclNum == info.compThisArg) + return; + bool isBorn = ((tree->gtFlags & GTF_VAR_DEF) != 0 && (tree->gtFlags & GTF_VAR_USEASG) == 0); // if it's "x <op>= ..." then variable "x" must have had a previous, original, site to be born. bool isDying = ((tree->gtFlags & GTF_VAR_DEATH) != 0); diff --git a/src/jit/lsra.cpp b/src/jit/lsra.cpp index 78815633cc..c7a91bb787 100644 --- a/src/jit/lsra.cpp +++ b/src/jit/lsra.cpp @@ -2250,6 +2250,7 @@ LinearScan::setLastUses(BasicBlock * block) { unsigned varNum = currentRefPosition->getInterval()->varNum; unsigned varIndex = currentRefPosition->getInterval()->getVarIndex(compiler); + // We should always have a tree node for a localVar, except for the "special" RefPositions. GenTreePtr tree = currentRefPosition->treeNode; assert(tree != nullptr || currentRefPosition->refType == RefTypeExpUse || currentRefPosition->refType == RefTypeDummyDef); @@ -2285,7 +2286,8 @@ LinearScan::setLastUses(BasicBlock * block) } } - if (currentRefPosition->refType == RefTypeDef || currentRefPosition->refType == RefTypeDummyDef) + if ((currentRefPosition->refType == RefTypeDef || currentRefPosition->refType == RefTypeDummyDef) && + (varNum != keepAliveVarNum)) { VarSetOps::RemoveElemD(compiler, temp, varIndex); } |