Skip to content

Commit

Permalink
Consolidate importer spilling code V3 (dotnet#73233)
Browse files Browse the repository at this point in the history
* Consolidate importer spilling code V2 (dotnet#72744)

* Consolidate importer spilling code (dotnet#72291)

* Add tests

* Fix losing GLOB_REF on the LHS

The comment states we don't need it, which is incorrect.

Diffs are improvements because we block forward substitution of
calls into "ASG(BLK(ADDR(LCL_VAR<field>, ...)))", which allows
morph to leave the "can be replaced with its field" local alone.

* Prospective fix

Spill "glob refs" on stores to "aliased" locals.

* Delete now-not-necessary code

* Fix up asserts

* Clean out '(unsigned)CHECK_SPILL_ALL/NONE' casts

* Don't manually spill for 'st[s]fld'

* Revert 'Clean out '(unsigned)CHECK_SPILL_ALL/NONE' casts'

* Fix assignments done via return buffers

The mistake in logic was that the only trees which could modify
unaliased locals are assignments, which is not true, calls can
do that as well.

One day we will move the return buffer handling out of importer,
but until then, special handling is required.

An alternative fix would have been to bring back the explicit
"impSpillLclRefs" to "stloc/starg" code, but that would contradict
the overall goal of consolidating the spilling logic.

* Account for "implicit" GLOB_REFs

Indirections off of "aliased" locals must be treated as global references
even as they are not marked with GLOB_REF.

Ideally, the importer would be designed such that this special-casing would
not be necessary (i. e. all indirections are initially GLOB_REF and then
morph "cleans" the flags once address-exposure info has been determined),
but we're not there yet, and it is questionable whether we'll get to that
point in any sort of near future.
  • Loading branch information
SingleAccretion committed Sep 6, 2022
1 parent 1a6ac1f commit 77613dc
Show file tree
Hide file tree
Showing 4 changed files with 518 additions and 158 deletions.
9 changes: 3 additions & 6 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -3810,11 +3810,8 @@ class Compiler
Statement* impLastStmt; // The last statement for the current BB.

public:
enum
{
CHECK_SPILL_ALL = -1,
CHECK_SPILL_NONE = -2
};
static const unsigned CHECK_SPILL_ALL = static_cast<unsigned>(-1);
static const unsigned CHECK_SPILL_NONE = static_cast<unsigned>(-2);

void impBeginTreeList();
void impEndTreeList(BasicBlock* block, Statement* firstStmt, Statement* lastStmt);
Expand Down Expand Up @@ -4014,7 +4011,7 @@ class Compiler
void impSpillSpecialSideEff();
void impSpillSideEffect(bool spillGlobEffects, unsigned chkLevel DEBUGARG(const char* reason));
void impSpillSideEffects(bool spillGlobEffects, unsigned chkLevel DEBUGARG(const char* reason));
void impSpillLclRefs(unsigned lclNum);
void impSpillLclRefs(unsigned lclNum, unsigned chkLevel);

BasicBlock* impPushCatchArgOnStack(BasicBlock* hndBlk, CORINFO_CLASS_HANDLE clsHnd, bool isSingleBlockFilter);

Expand Down
Loading

0 comments on commit 77613dc

Please sign in to comment.