Skip to content

Commit

Permalink
JIT: Update type when return temp is freshly created (#111948)
Browse files Browse the repository at this point in the history
* Teach the JIT about System.SZArrayHelper.GetEnumerator

* A better approach

* Format JIT

* Use lvaInlineeReturnSpillTempFreshlyCreated

* Update src/coreclr/jit/fgopt.cpp

Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>

* Update assertion

* Nit

* Nit

* Address review feedbacks

* Make the check always available

* Deploy a fix for COM objects

* Revert "Deploy a fix for COM objects"

This reverts commit e94839b.

* The real fix

* Address review feedback

* Fix typo

---------

Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
Co-authored-by: Andy Ayers <andya@microsoft.com>
  • Loading branch information
3 people authored Feb 1, 2025
1 parent ad7b829 commit 44167e0
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 14 deletions.
4 changes: 3 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -4091,6 +4091,8 @@ class Compiler
unsigned lvaInlineeReturnSpillTemp = BAD_VAR_NUM; // The temp to spill the non-VOID return expression
// in case there are multiple BBJ_RETURN blocks in the inlinee
// or if the inlinee has GC ref locals.

bool lvaInlineeReturnSpillTempFreshlyCreated = false; // True if the temp was freshly created for the inlinee return

#if FEATURE_FIXED_OUT_ARGS
unsigned lvaOutgoingArgSpaceVar = BAD_VAR_NUM; // var that represents outgoing argument space
Expand Down Expand Up @@ -4355,7 +4357,7 @@ class Compiler
// If the local is TYP_REF, set or update the associated class information.
void lvaSetClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool isExact = false);
void lvaSetClass(unsigned varNum, GenTree* tree, CORINFO_CLASS_HANDLE stackHandle = nullptr);
void lvaUpdateClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool isExact = false);
void lvaUpdateClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool isExact = false, bool singleDefOnly = true);
void lvaUpdateClass(unsigned varNum, GenTree* tree, CORINFO_CLASS_HANDLE stackHandle = nullptr);

#define MAX_NumOfFieldsInPromotableStruct 4 // Maximum number of fields in promotable struct
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3562,6 +3562,8 @@ void Compiler::fgFindBasicBlocks()
lvaSetClass(lvaInlineeReturnSpillTemp, retClassHnd);
}
}

lvaInlineeReturnSpillTempFreshlyCreated = true;
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/coreclr/jit/fgopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,17 +309,19 @@ PhaseStatus Compiler::fgPostImportationCleanup()
{
// Update type of return spill temp if we have gathered
// better info when importing the inlinee, and the return
// spill temp is single def.
// spill temp is single def or was freshly created for this inlinee
if (fgNeedReturnSpillTemp())
{
CORINFO_CLASS_HANDLE retExprClassHnd = impInlineInfo->retExprClassHnd;
if (retExprClassHnd != nullptr)
{
LclVarDsc* returnSpillVarDsc = lvaGetDesc(lvaInlineeReturnSpillTemp);

if ((returnSpillVarDsc->lvType == TYP_REF) && returnSpillVarDsc->lvSingleDef)
if (returnSpillVarDsc->lvType == TYP_REF &&
(returnSpillVarDsc->lvSingleDef || lvaInlineeReturnSpillTempFreshlyCreated))
{
lvaUpdateClass(lvaInlineeReturnSpillTemp, retExprClassHnd, impInlineInfo->retExprClassHndIsExact);
lvaUpdateClass(lvaInlineeReturnSpillTemp, retExprClassHnd, impInlineInfo->retExprClassHndIsExact,
false);
}
}
}
Expand Down
20 changes: 14 additions & 6 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11076,13 +11076,21 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
impInlineInfo->retExprClassHnd = returnClsHnd;
impInlineInfo->retExprClassHndIsExact = isExact;
}
else if (impInlineInfo->retExprClassHnd != returnClsHnd)
else
{
// This return site type differs from earlier seen sites,
// so reset the info and we'll fall back to using the method's
// declared return type for the return spill temp.
impInlineInfo->retExprClassHnd = nullptr;
impInlineInfo->retExprClassHndIsExact = false;
if (impInlineInfo->retExprClassHnd != returnClsHnd)
{
// This return site type differs from earlier seen sites,
// so reset the info and we'll fall back to using the method's
// declared return type for the return spill temp.
impInlineInfo->retExprClassHnd = nullptr;
impInlineInfo->retExprClassHndIsExact = false;
}
else
{
// Same return type, but we may need to update exactness.
impInlineInfo->retExprClassHndIsExact &= isExact;
}
}
}

Expand Down
13 changes: 9 additions & 4 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3601,6 +3601,7 @@ void Compiler::lvaSetClass(unsigned varNum, GenTree* tree, CORINFO_CLASS_HANDLE
// varNum -- number of the variable
// clsHnd -- class handle to use in set or update
// isExact -- true if class is known exactly
// singleDefOnly -- true if we should only update single-def locals
//
// Notes:
//
Expand All @@ -3618,7 +3619,7 @@ void Compiler::lvaSetClass(unsigned varNum, GenTree* tree, CORINFO_CLASS_HANDLE
// for shared code, so ensuring this is so is currently not
// possible.

void Compiler::lvaUpdateClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool isExact)
void Compiler::lvaUpdateClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool isExact, bool singleDefOnly)
{
assert(varNum < lvaCount);

Expand All @@ -3631,8 +3632,12 @@ void Compiler::lvaUpdateClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool
// We should already have a class
assert(varDsc->lvClassHnd != NO_CLASS_HANDLE);

// We should only be updating classes for single-def locals.
assert(varDsc->lvSingleDef);
// We should only be updating classes for single-def locals if requested
if (singleDefOnly && !varDsc->lvSingleDef)
{
assert(!"Updating class for multi-def local");
return;
}

// Now see if we should update.
//
Expand Down Expand Up @@ -3679,7 +3684,7 @@ void Compiler::lvaUpdateClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool
}

//------------------------------------------------------------------------
// lvaUpdateClass: Uupdate class information for a local var from a tree
// lvaUpdateClass: Update class information for a local var from a tree
// or stack type
//
// Arguments:
Expand Down

0 comments on commit 44167e0

Please sign in to comment.