JIT: refactor how opaque VNs are represented (#55853)
authorAndy Ayers <andya@microsoft.com>
Mon, 19 Jul 2021 15:39:28 +0000 (08:39 -0700)
committerGitHub <noreply@github.com>
Mon, 19 Jul 2021 15:39:28 +0000 (08:39 -0700)
Use a unary function to capture opaque VN loop dependence, rather than factoring
that out as part of the owning chunk. As a result the VN chunk data no longer
has a per-loop component (no dependence on MAX_LOOP_NUM).

This gives us the flexibility to address #54118 by doing something similar for
`MapUpdate` VNs.

src/coreclr/jit/optimizer.cpp
src/coreclr/jit/valuenum.cpp
src/coreclr/jit/valuenum.h
src/coreclr/jit/valuenumfuncs.h

index 3f8e1ee99cb0c73b83ddf3fdcdb29905c635b5d6..340dd64e7e6327ac77b264effa9de1d812ece301 100644 (file)
@@ -6567,6 +6567,11 @@ bool Compiler::optVNIsLoopInvariant(ValueNum vn, unsigned lnum, VNToBoolMap* loo
             BasicBlock* defnBlk = reinterpret_cast<BasicBlock*>(vnStore->ConstantValue<ssize_t>(funcApp.m_args[0]));
             res                 = !optLoopContains(lnum, defnBlk->bbNatLoopNum);
         }
+        else if (funcApp.m_func == VNF_MemOpaque)
+        {
+            const unsigned vnLoopNum = funcApp.m_args[0];
+            res                      = !optLoopContains(lnum, vnLoopNum);
+        }
         else
         {
             for (unsigned i = 0; i < funcApp.m_arity; i++)
@@ -6582,21 +6587,6 @@ bool Compiler::optVNIsLoopInvariant(ValueNum vn, unsigned lnum, VNToBoolMap* loo
             }
         }
     }
-    else
-    {
-        // Non-function "new, unique" VN's may be annotated with the loop nest where
-        // their definition occurs.
-        BasicBlock::loopNumber vnLoopNum = vnStore->LoopOfVN(vn);
-
-        if (vnLoopNum == BasicBlock::MAX_LOOP_NUM)
-        {
-            res = false;
-        }
-        else
-        {
-            res = !optLoopContains(lnum, vnLoopNum);
-        }
-    }
 
     loopVnInvariantCache->Set(vn, res);
     return res;
index abda97b17e30b44521c439193b7ddcf1db5460fd..8bc2ace8f6b56f8357ab779e22b5c1537aa4ccb4 100644 (file)
@@ -454,7 +454,7 @@ ValueNumStore::ValueNumStore(Compiler* comp, CompAllocator alloc)
     // We have no current allocation chunks.
     for (unsigned i = 0; i < TYP_COUNT; i++)
     {
-        for (unsigned j = CEA_None; j <= CEA_Count + BasicBlock::MAX_LOOP_NUM; j++)
+        for (unsigned j = CEA_Const; j <= CEA_Count; j++)
         {
             m_curAllocChunk[i][j] = NoChunk;
         }
@@ -466,8 +466,7 @@ ValueNumStore::ValueNumStore(Compiler* comp, CompAllocator alloc)
     }
     // We will reserve chunk 0 to hold some special constants, like the constant NULL, the "exception" value, and the
     // "zero map."
-    Chunk* specialConstChunk =
-        new (m_alloc) Chunk(m_alloc, &m_nextChunkBase, TYP_REF, CEA_Const, BasicBlock::MAX_LOOP_NUM);
+    Chunk* specialConstChunk = new (m_alloc) Chunk(m_alloc, &m_nextChunkBase, TYP_REF, CEA_Const);
     specialConstChunk->m_numUsed +=
         SRC_NumSpecialRefConsts; // Implicitly allocate 0 ==> NULL, and 1 ==> Exception, 2 ==> ZeroMap.
     ChunkNum cn = m_chunks.Push(specialConstChunk);
@@ -1552,17 +1551,12 @@ bool ValueNumStore::IsSharedStatic(ValueNum vn)
     return GetVNFunc(vn, &funcAttr) && (s_vnfOpAttribs[funcAttr.m_func] & VNFOA_SharedStatic) != 0;
 }
 
-ValueNumStore::Chunk::Chunk(CompAllocator          alloc,
-                            ValueNum*              pNextBaseVN,
-                            var_types              typ,
-                            ChunkExtraAttribs      attribs,
-                            BasicBlock::loopNumber loopNum)
-    : m_defs(nullptr), m_numUsed(0), m_baseVN(*pNextBaseVN), m_typ(typ), m_attribs(attribs), m_loopNum(loopNum)
+ValueNumStore::Chunk::Chunk(CompAllocator alloc, ValueNum* pNextBaseVN, var_types typ, ChunkExtraAttribs attribs)
+    : m_defs(nullptr), m_numUsed(0), m_baseVN(*pNextBaseVN), m_typ(typ), m_attribs(attribs)
 {
     // Allocate "m_defs" here, according to the typ/attribs pair.
     switch (attribs)
     {
-        case CEA_None:
         case CEA_NotAField:
             break; // Nothing to do.
         case CEA_Const:
@@ -1619,26 +1613,11 @@ ValueNumStore::Chunk::Chunk(CompAllocator          alloc,
     *pNextBaseVN += ChunkSize;
 }
 
-ValueNumStore::Chunk* ValueNumStore::GetAllocChunk(var_types              typ,
-                                                   ChunkExtraAttribs      attribs,
-                                                   BasicBlock::loopNumber loopNum)
+ValueNumStore::Chunk* ValueNumStore::GetAllocChunk(var_types typ, ChunkExtraAttribs attribs)
 {
     Chunk*   res;
-    unsigned index;
-    if (loopNum == BasicBlock::MAX_LOOP_NUM)
-    {
-        // Loop nest is unknown/irrelevant for this VN.
-        index = attribs;
-    }
-    else
-    {
-        // Loop nest is interesting.  Since we know this is only true for unique VNs, we know attribs will
-        // be CEA_None and can just index based on loop number.
-        noway_assert(attribs == CEA_None);
-        // Map NOT_IN_LOOP -> BasicBlock::MAX_LOOP_NUM to make the index range contiguous [0..BasicBlock::MAX_LOOP_NUM]
-        index = CEA_Count + (loopNum == BasicBlock::NOT_IN_LOOP ? BasicBlock::MAX_LOOP_NUM : loopNum);
-    }
-    ChunkNum cn = m_curAllocChunk[typ][index];
+    unsigned index = attribs;
+    ChunkNum cn    = m_curAllocChunk[typ][index];
     if (cn != NoChunk)
     {
         res = m_chunks.Get(cn);
@@ -1648,7 +1627,7 @@ ValueNumStore::Chunk* ValueNumStore::GetAllocChunk(var_types              typ,
         }
     }
     // Otherwise, must allocate a new one.
-    res                         = new (m_alloc) Chunk(m_alloc, &m_nextChunkBase, typ, attribs, loopNum);
+    res                         = new (m_alloc) Chunk(m_alloc, &m_nextChunkBase, typ, attribs);
     cn                          = m_chunks.Push(res);
     m_curAllocChunk[typ][index] = cn;
     return res;
@@ -1782,10 +1761,13 @@ ValueNum ValueNumStore::VNForHandle(ssize_t cnsVal, GenTreeFlags handleFlags)
     }
     else
     {
-        Chunk*   c                                                = GetAllocChunk(TYP_I_IMPL, CEA_Handle);
-        unsigned offsetWithinChunk                                = c->AllocVN();
-        res                                                       = c->m_baseVN + offsetWithinChunk;
-        reinterpret_cast<VNHandle*>(c->m_defs)[offsetWithinChunk] = handle;
+        Chunk* const    c                 = GetAllocChunk(TYP_I_IMPL, CEA_Handle);
+        unsigned const  offsetWithinChunk = c->AllocVN();
+        VNHandle* const chunkSlots        = reinterpret_cast<VNHandle*>(c->m_defs);
+
+        chunkSlots[offsetWithinChunk] = handle;
+        res                           = c->m_baseVN + offsetWithinChunk;
+
         GetHandleMap()->Set(handle, res);
         return res;
     }
@@ -1886,10 +1868,12 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func)
     if (!GetVNFunc0Map()->Lookup(func, &resultVN))
     {
         // Allocate a new ValueNum for 'func'
-        Chunk*   c                                              = GetAllocChunk(typ, CEA_Func0);
-        unsigned offsetWithinChunk                              = c->AllocVN();
-        resultVN                                                = c->m_baseVN + offsetWithinChunk;
-        reinterpret_cast<VNFunc*>(c->m_defs)[offsetWithinChunk] = func;
+        Chunk* const   c                 = GetAllocChunk(typ, CEA_Func0);
+        unsigned const offsetWithinChunk = c->AllocVN();
+        VNFunc* const  chunkSlots        = reinterpret_cast<VNFunc*>(c->m_defs);
+
+        chunkSlots[offsetWithinChunk] = func;
+        resultVN                      = c->m_baseVN + offsetWithinChunk;
         GetVNFunc0Map()->Set(func, resultVN);
     }
     return resultVN;
@@ -1911,6 +1895,7 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func)
 //
 ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN)
 {
+    assert(func != VNF_MemOpaque);
     assert(arg0VN == VNNormalValue(arg0VN)); // Arguments don't carry exceptions.
 
     // Try to perform constant-folding.
@@ -1922,16 +1907,18 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN)
     ValueNum resultVN;
 
     // Have we already assigned a ValueNum for 'func'('arg0VN') ?
-    //
     VNDefFunc1Arg fstruct(func, arg0VN);
     if (!GetVNFunc1Map()->Lookup(fstruct, &resultVN))
     {
         // Otherwise, Allocate a new ValueNum for 'func'('arg0VN')
         //
-        Chunk*   c                                                     = GetAllocChunk(typ, CEA_Func1);
-        unsigned offsetWithinChunk                                     = c->AllocVN();
-        resultVN                                                       = c->m_baseVN + offsetWithinChunk;
-        reinterpret_cast<VNDefFunc1Arg*>(c->m_defs)[offsetWithinChunk] = fstruct;
+        Chunk* const         c                 = GetAllocChunk(typ, CEA_Func1);
+        unsigned const       offsetWithinChunk = c->AllocVN();
+        VNDefFunc1Arg* const chunkSlots        = reinterpret_cast<VNDefFunc1Arg*>(c->m_defs);
+
+        chunkSlots[offsetWithinChunk] = fstruct;
+        resultVN                      = c->m_baseVN + offsetWithinChunk;
+
         // Record 'resultVN' in the Func1Map
         GetVNFunc1Map()->Set(fstruct, resultVN);
     }
@@ -2047,10 +2034,12 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN, V
             {
                 // Otherwise, Allocate a new ValueNum for 'func'('arg0VN','arg1VN')
                 //
-                Chunk*   c                                                     = GetAllocChunk(typ, CEA_Func2);
-                unsigned offsetWithinChunk                                     = c->AllocVN();
-                resultVN                                                       = c->m_baseVN + offsetWithinChunk;
-                reinterpret_cast<VNDefFunc2Arg*>(c->m_defs)[offsetWithinChunk] = fstruct;
+                Chunk* const         c                 = GetAllocChunk(typ, CEA_Func2);
+                unsigned const       offsetWithinChunk = c->AllocVN();
+                VNDefFunc2Arg* const chunkSlots        = reinterpret_cast<VNDefFunc2Arg*>(c->m_defs);
+
+                chunkSlots[offsetWithinChunk] = fstruct;
+                resultVN                      = c->m_baseVN + offsetWithinChunk;
                 // Record 'resultVN' in the Func2Map
                 GetVNFunc2Map()->Set(fstruct, resultVN);
             }
@@ -2108,10 +2097,13 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN, V
     {
         // Otherwise, Allocate a new ValueNum for 'func'('arg0VN','arg1VN','arg2VN')
         //
-        Chunk*   c                                                     = GetAllocChunk(typ, CEA_Func3);
-        unsigned offsetWithinChunk                                     = c->AllocVN();
-        resultVN                                                       = c->m_baseVN + offsetWithinChunk;
-        reinterpret_cast<VNDefFunc3Arg*>(c->m_defs)[offsetWithinChunk] = fstruct;
+        Chunk* const         c                 = GetAllocChunk(typ, CEA_Func3);
+        unsigned const       offsetWithinChunk = c->AllocVN();
+        VNDefFunc3Arg* const chunkSlots        = reinterpret_cast<VNDefFunc3Arg*>(c->m_defs);
+
+        chunkSlots[offsetWithinChunk] = fstruct;
+        resultVN                      = c->m_baseVN + offsetWithinChunk;
+
         // Record 'resultVN' in the Func3Map
         GetVNFunc3Map()->Set(fstruct, resultVN);
     }
@@ -2156,10 +2148,13 @@ ValueNum ValueNumStore::VNForFunc(
     {
         // Otherwise, Allocate a new ValueNum for 'func'('arg0VN','arg1VN','arg2VN','arg3VN')
         //
-        Chunk*   c                                                     = GetAllocChunk(typ, CEA_Func4);
-        unsigned offsetWithinChunk                                     = c->AllocVN();
-        resultVN                                                       = c->m_baseVN + offsetWithinChunk;
-        reinterpret_cast<VNDefFunc4Arg*>(c->m_defs)[offsetWithinChunk] = fstruct;
+        Chunk* const         c                 = GetAllocChunk(typ, CEA_Func4);
+        unsigned const       offsetWithinChunk = c->AllocVN();
+        VNDefFunc4Arg* const chunkSlots        = reinterpret_cast<VNDefFunc4Arg*>(c->m_defs);
+
+        chunkSlots[offsetWithinChunk] = fstruct;
+        resultVN                      = c->m_baseVN + offsetWithinChunk;
+
         // Record 'resultVN' in the Func4Map
         GetVNFunc4Map()->Set(fstruct, resultVN);
     }
@@ -2465,10 +2460,13 @@ TailCall:
         if (!GetVNFunc2Map()->Lookup(fstruct, &res))
         {
             // Otherwise, assign a new VN for the function application.
-            Chunk*   c                                                     = GetAllocChunk(typ, CEA_Func2);
-            unsigned offsetWithinChunk                                     = c->AllocVN();
-            res                                                            = c->m_baseVN + offsetWithinChunk;
-            reinterpret_cast<VNDefFunc2Arg*>(c->m_defs)[offsetWithinChunk] = fstruct;
+            Chunk* const         c                 = GetAllocChunk(typ, CEA_Func2);
+            unsigned const       offsetWithinChunk = c->AllocVN();
+            VNDefFunc2Arg* const chunkSlots        = reinterpret_cast<VNDefFunc2Arg*>(c->m_defs);
+
+            chunkSlots[offsetWithinChunk] = fstruct;
+            res                           = c->m_baseVN + offsetWithinChunk;
+
             GetVNFunc2Map()->Set(fstruct, res);
         }
         return res;
@@ -3675,12 +3673,17 @@ ValueNum ValueNumStore::VNForExpr(BasicBlock* block, var_types typ)
         loopNum = block->bbNatLoopNum;
     }
 
-    // We always allocate a new, unique VN in this call.
-    // The 'typ' is used to partition the allocation of VNs into different chunks.
-    Chunk*   c                 = GetAllocChunk(typ, CEA_None, loopNum);
-    unsigned offsetWithinChunk = c->AllocVN();
-    ValueNum result            = c->m_baseVN + offsetWithinChunk;
-    return result;
+    // VNForFunc(typ, func, vn) but bypasses looking in the cache
+    //
+    VNDefFunc1Arg        fstruct(VNF_MemOpaque, loopNum);
+    Chunk* const         c                 = GetAllocChunk(typ, CEA_Func1);
+    unsigned const       offsetWithinChunk = c->AllocVN();
+    VNDefFunc1Arg* const chunkSlots        = reinterpret_cast<VNDefFunc1Arg*>(c->m_defs);
+
+    chunkSlots[offsetWithinChunk] = fstruct;
+
+    ValueNum resultVN = c->m_baseVN + offsetWithinChunk;
+    return resultVN;
 }
 
 ValueNum ValueNumStore::VNApplySelectors(ValueNumKind  vnk,
@@ -4343,27 +4346,25 @@ var_types ValueNumStore::TypeOfVN(ValueNum vn)
 }
 
 //------------------------------------------------------------------------
-// LoopOfVN: If the given value number is an opaque one associated with a particular
-//    expression in the IR, give the loop number where the expression occurs; otherwise,
-//    returns BasicBlock::MAX_LOOP_NUM.
+// LoopOfVN: If the given value number is VNF_MemOpaque, return
+//    the loop number where the memory update occurs, otherwise returns MAX_LOOP_NUM.
 //
 // Arguments:
 //    vn - Value number to query
 //
 // Return Value:
-//    The correspondingblock's bbNatLoopNum, which may be BasicBlock::NOT_IN_LOOP.
-//    Returns BasicBlock::MAX_LOOP_NUM if this VN is not an opaque value number associated with
-//    a particular expression/location in the IR.
-
+//    The memory loop number, which may be BasicBlock::NOT_IN_LOOP.
+//    Returns BasicBlock::MAX_LOOP_NUM if this VN is not a memory value number.
+//
 BasicBlock::loopNumber ValueNumStore::LoopOfVN(ValueNum vn)
 {
-    if (vn == NoVN)
+    VNFuncApp funcApp;
+    if (GetVNFunc(vn, &funcApp) && (funcApp.m_func == VNF_MemOpaque))
     {
-        return BasicBlock::MAX_LOOP_NUM;
+        return (BasicBlock::loopNumber)funcApp.m_args[0];
     }
 
-    Chunk* c = m_chunks.GetNoExpand(GetChunkNum(vn));
-    return c->m_loopNum;
+    return BasicBlock::MAX_LOOP_NUM;
 }
 
 bool ValueNumStore::IsVNConstant(ValueNum vn)
@@ -5554,6 +5555,9 @@ void ValueNumStore::vnDump(Compiler* comp, ValueNum vn, bool isPtr)
             case VNF_ValWithExc:
                 vnDumpValWithExc(comp, &funcApp);
                 break;
+            case VNF_MemOpaque:
+                vnDumpMemOpaque(comp, &funcApp);
+                break;
 #ifdef FEATURE_SIMD
             case VNF_SimdType:
                 vnDumpSimdType(comp, &funcApp);
@@ -5712,6 +5716,25 @@ void ValueNumStore::vnDumpMapStore(Compiler* comp, VNFuncApp* mapStore)
     printf("]");
 }
 
+void ValueNumStore::vnDumpMemOpaque(Compiler* comp, VNFuncApp* memOpaque)
+{
+    assert(memOpaque->m_func == VNF_MemOpaque); // Precondition.
+    const unsigned loopNum = memOpaque->m_args[0];
+
+    if (loopNum == BasicBlock::NOT_IN_LOOP)
+    {
+        printf("MemOpaque:NotInLoop");
+    }
+    else if (loopNum == BasicBlock::MAX_LOOP_NUM)
+    {
+        printf("MemOpaque:Indeterminate");
+    }
+    else
+    {
+        printf("MemOpaque:L%02u", loopNum);
+    }
+}
+
 #ifdef FEATURE_SIMD
 void ValueNumStore::vnDumpSimdType(Compiler* comp, VNFuncApp* simdType)
 {
index 481622a8198b7ae6680386c8242543beb6204320..b64d19993fa721ddb9da9cfd667da25d0d520613 100644 (file)
@@ -880,6 +880,10 @@ public:
     // Prints a representation of a MapStore operation on standard out.
     void vnDumpMapStore(Compiler* comp, VNFuncApp* mapStore);
 
+    // Requires "memOpaque" to be a mem opaque VNFuncApp
+    // Prints a representation of a MemOpaque state on standard out.
+    void vnDumpMemOpaque(Compiler* comp, VNFuncApp* memOpaque);
+
     // Requires "valWithExc" to be a value with an exeception set VNFuncApp.
     // Prints a representation of the exeception set on standard out.
     void vnDumpValWithExc(Compiler* comp, VNFuncApp* valWithExc);
@@ -935,7 +939,6 @@ private:
 
     enum ChunkExtraAttribs : BYTE
     {
-        CEA_None,      // No extra attributes.
         CEA_Const,     // This chunk contains constant values.
         CEA_Handle,    // This chunk contains handle constants.
         CEA_NotAField, // This chunk contains "not a field" values.
@@ -961,18 +964,12 @@ private:
         ValueNum m_baseVN;
 
         // The common attributes of this chunk.
-        var_types              m_typ;
-        ChunkExtraAttribs      m_attribs;
-        BasicBlock::loopNumber m_loopNum;
+        var_types         m_typ;
+        ChunkExtraAttribs m_attribs;
 
-        // Initialize a chunk, starting at "*baseVN", for the given "typ", "attribs", and "loopNum" (using "alloc" for
-        // allocations).
+        // Initialize a chunk, starting at "*baseVN", for the given "typ", and "attribs", using "alloc" for allocations.
         // (Increments "*baseVN" by ChunkSize.)
-        Chunk(CompAllocator          alloc,
-              ValueNum*              baseVN,
-              var_types              typ,
-              ChunkExtraAttribs      attribs,
-              BasicBlock::loopNumber loopNum);
+        Chunk(CompAllocator alloc, ValueNum* baseVN, var_types typ, ChunkExtraAttribs attribs);
 
         // Requires that "m_numUsed < ChunkSize."  Returns the offset of the allocated VN within the chunk; the
         // actual VN is this added to the "m_baseVN" of the chunk.
@@ -1121,17 +1118,14 @@ private:
     JitExpandArrayStack<Chunk*> m_chunks;
 
     // These entries indicate the current allocation chunk, if any, for each valid combination of <var_types,
-    // ChunkExtraAttribute, loopNumber>.  Valid combinations require attribs==CEA_None or
-    // loopNum==BasicBlock::MAX_LOOP_NUM.
+    // ChunkExtraAttribute>.
     // If the value is NoChunk, it indicates that there is no current allocation chunk for that pair, otherwise
     // it is the index in "m_chunks" of a chunk with the given attributes, in which the next allocation should
     // be attempted.
-    ChunkNum m_curAllocChunk[TYP_COUNT][CEA_Count + BasicBlock::MAX_LOOP_NUM + 1];
+    ChunkNum m_curAllocChunk[TYP_COUNT][CEA_Count + 1];
 
     // Returns a (pointer to a) chunk in which a new value number may be allocated.
-    Chunk* GetAllocChunk(var_types              typ,
-                         ChunkExtraAttribs      attribs,
-                         BasicBlock::loopNumber loopNum = BasicBlock::MAX_LOOP_NUM);
+    Chunk* GetAllocChunk(var_types typ, ChunkExtraAttribs attribs);
 
     // First, we need mechanisms for mapping from constants to value numbers.
     // For small integers, we'll use an array.
index 2ddfff34dfa4550eb65740a78f0093fea2e3ee11..af53a7b1e0574c660febd2e244d7d36b87040c1f 100644 (file)
@@ -6,6 +6,7 @@
 // <is-shared-static>)
 
 // clang-format off
+ValueNumFuncDef(MemOpaque, 1, false, false, false)  // Args: 0: loop num
 ValueNumFuncDef(MapStore, 3, false, false, false)   // Args: 0: map, 1: index (e. g. field handle), 2: value being stored.
 ValueNumFuncDef(MapSelect, 2, false, false, false)  // Args: 0: map, 1: key.