Always normalize stores when spilling lclVars.

In order to support the semantics of the MSIL evaluation stack, RyuJIT
normalizes all "small" lclVars (i.e. lclVars with a non-struct
type that is smaller than an int32) from the actual type of the lclVar
to int32. This normalization may be performed in one of two ways:
- the lclVar's may be normalized when the lclVar is used (load
  normalization)
- at any point at which the value is defined, the value being stored to
  the lclVar is normalized (store normalization)

Store normalization must not be used if the lclVar is aliasable, since
the contents of the upper bytes for that lclVar's slot may be modified
by some other alias. Load normalization must be used if the lclVar is
a method parameter, as there is no guarantee that the caller has
properly nomralized the arguments it has passed. As such, any lclVar
that is either aliasable or is a method parameter is load-normalized,
and any other lclVar is store-normalized. lclVars that are
store-normalized are assumed to be represented as properly zero- or
sign-extended 4-byte values at all times, and so can simply be loaded
as int32s.

This picture becomes somewhat more subtle after register allocation,
however: once a lclVar has been enregistered, the contents of its
register are guaranteed to be properly zero- or sign-extended for the
purposes of store normalization, and in order for the lclVar to have
been enregistered in the first place, it must not have been aliasable.
As such, all enregistered variables may be store-normalized (as well as
load-normalized, if they represent method parameters). This has a
particularly significant impact on spills, as it implies that even if
the lclVar being spilled is not normally considered store-normalized, it
may be considered so for the purposes of the spill. This is advantageous
for architectures such as x86, as it allows a small lclVar to be
successfully spilled from a register that is not addressable at the size
of the lclVar's type. This change switches both LSRA and the code
generator to always use the actual type of a spilled lclVar for the
corresponding store rather than making this behavior conditional on
whether or not the lclVar is store-normalized.

Fixes #7236.

diff --git a/src/jit/codegenlinear.cpp b/src/jit/codegenlinear.cpp
index 0d99ee0..121b4ff 100644 (file)
--- a/src/jit/codegenlinear.cpp
+++ b/src/jit/codegenlinear.cpp
@@ -710,12 +710,12 @@ void CodeGen::genSpillVar(GenTreePtr tree)
     bool needsSpill = ((tree->gtFlags & GTF_VAR_DEF) == 0 && varDsc->lvIsInReg());
     if (needsSpill)
     {
-        var_types lclTyp = varDsc->TypeGet();
-        if (varDsc->lvNormalizeOnStore())
-        {
-            lclTyp = genActualType(lclTyp);
-        }
-        emitAttr size = emitTypeSize(lclTyp);
+        // In order for a lclVar to have been allocated to a register, it must not have been aliasable, and can
+        // therefore be store-normalized (rather than load-normalized). In fact, not performing store normalization
+        // can lead to problems on architectures where a lclVar may be allocated to a register that is not
+        // addressable at the granularity of the lclVar's defined type (e.g. x86).
+        var_types lclTyp = genActualType(varDsc->TypeGet());
+        emitAttr  size   = emitTypeSize(lclTyp);
 
         bool restoreRegVar = false;
         if (tree->gtOper == GT_REG_VAR)

diff --git a/src/jit/lsra.cpp b/src/jit/lsra.cpp
index 774d1fb..6c6ea97 100644 (file)
--- a/src/jit/lsra.cpp
+++ b/src/jit/lsra.cpp
@@ -8423,23 +8423,10 @@ void LinearScan::resolveRegisters()
                     varDsc->lvArgInitReg = initialReg;
                     JITDUMP("  Set V%02u argument initial register to %s\n", lclNum, getRegName(initialReg));
                 }
-                if (!varDsc->lvIsRegArg)
-                {
-                    // stack arg
-                    if (compiler->lvaIsFieldOfDependentlyPromotedStruct(varDsc))
-                    {
-                        if (sourceReg != initialReg)
-                        {
-                            // The code generator won't initialize struct
-                            // fields, so we have to do that if it's not already
-                            // where it belongs.
-                            assert(interval->isStructField);
-                            JITDUMP("  Move struct field param V%02u from %s to %s\n", lclNum, getRegName(sourceReg),
-                                    getRegName(initialReg));
-                            insertMove(insertionBlock, insertionPoint, lclNum, sourceReg, initialReg);
-                        }
-                    }
-                }
+
+                // Stack args that are part of dependently-promoted structs should never be register candidates (see
+                // LinearScan::isRegCandidate).
+                assert(varDsc->lvIsRegArg || !compiler->lvaIsFieldOfDependentlyPromotedStruct(varDsc));
             }
 
             // If lvRegNum is REG_STK, that means that either no register
@@ -8573,6 +8560,8 @@ void LinearScan::insertMove(
     BasicBlock* block, GenTreePtr insertionPoint, unsigned lclNum, regNumber fromReg, regNumber toReg)
 {
     LclVarDsc* varDsc = compiler->lvaTable + lclNum;
+    // the lclVar must be a register candidate
+    assert(isRegCandidate(varDsc));
     // One or both MUST be a register
     assert(fromReg != REG_STK || toReg != REG_STK);
     // They must not be the same register.
@@ -8581,20 +8570,22 @@ void LinearScan::insertMove(
     // This var can't be marked lvRegister now
     varDsc->lvRegNum = REG_STK;
 
-    var_types lclTyp = varDsc->TypeGet();
-    if (varDsc->lvNormalizeOnStore())
-    {
-        lclTyp = genActualType(lclTyp);
-    }
-    GenTreePtr src              = compiler->gtNewLclvNode(lclNum, lclTyp);
+    GenTreePtr src              = compiler->gtNewLclvNode(lclNum, varDsc->TypeGet());
     src->gtLsraInfo.isLsraAdded = true;
-    GenTreePtr top;
 
-    // If we are moving from STK to reg, mark the lclVar nodes with GTF_SPILLED
-    // Otherwise, if we are moving from reg to stack, mark it as GTF_SPILL
-    // Finally, for a reg-to-reg move, generate a GT_COPY
+    // There are three cases we need to handle:
+    // - We are loading a lclVar from the stack.
+    // - We are storing a lclVar to the stack.
+    // - We are copying a lclVar between registers.
+    //
+    // In the first and second cases, the lclVar node will be marked with GTF_SPILLED and GTF_SPILL, respetively.
+    // It is up to the code generator to ensure that any necessary normalization is done when loading or storing the
+    // lclVar's value.
+    //
+    // In the third case, we generate GT_COPY(GT_LCL_VAR) and type each node with the normalized type of the lclVar.
+    // This is safe because a lclVar is always normalized once it is in a register.
 
-    top = src;
+    GenTree* dst = src;
     if (fromReg == REG_STK)
     {
         src->gtFlags |= GTF_SPILLED;
@@ -8608,21 +8599,23 @@ void LinearScan::insertMove(
     }
     else
     {
-        top = new (compiler, GT_COPY) GenTreeCopyOrReload(GT_COPY, varDsc->TypeGet(), src);
+        // If wer 
+        var_types movType = genActualType(varDsc->TypeGet());
+        src->gtType       = movType;
+
+        dst = new (compiler, GT_COPY) GenTreeCopyOrReload(GT_COPY, movType, src);
         // This is the new home of the lclVar - indicate that by clearing the GTF_VAR_DEATH flag.
         // Note that if src is itself a lastUse, this will have no effect.
-        top->gtFlags &= ~(GTF_VAR_DEATH);
+        dst->gtFlags &= ~(GTF_VAR_DEATH);
         src->gtRegNum = fromReg;
         src->SetInReg();
-        top->gtRegNum                 = toReg;
-        src->gtNext                   = top;
-        top->gtPrev                   = src;
+        dst->gtRegNum                 = toReg;
         src->gtLsraInfo.isLocalDefUse = false;
-        top->gtLsraInfo.isLsraAdded   = true;
+        dst->gtLsraInfo.isLsraAdded   = true;
     }
-    top->gtLsraInfo.isLocalDefUse = true;
+    dst->gtLsraInfo.isLocalDefUse = true;
 
-    LIR::Range  treeRange  = LIR::SeqTree(compiler, top);
+    LIR::Range  treeRange  = LIR::SeqTree(compiler, dst);
     LIR::Range& blockRange = LIR::AsRange(block);
 
     if (insertionPoint != nullptr)