From: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> Date: Wed, 10 May 2023 04:57:43 +0000 (+0300) Subject: Delete rarely used bashing methods and add some documentation on bashing (#85925) X-Git-Tag: accepted/tizen/unified/riscv/20231226.055536~2338 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3d2cc5c34599e3cb86775d6618bf4b588fdcf361;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Delete rarely used bashing methods and add some documentation on bashing (#85925) --- diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 96a579cd51b..8e2e5ee16d3 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -1168,6 +1168,82 @@ inline void Compiler::fgUpdateConstTreeValueNumber(GenTree* tree) } } +inline GenTree* Compiler::gtNewKeepAliveNode(GenTree* op) +{ + GenTree* keepalive = gtNewOperNode(GT_KEEPALIVE, TYP_VOID, op); + + // Prevent both reordering and removal. Invalid optimizations of GC.KeepAlive are + // very subtle and hard to observe. Thus we are conservatively marking it with both + // GTF_CALL and GTF_GLOB_REF side-effects even though it may be more than strictly + // necessary. The conservative side-effects are unlikely to have negative impact + // on code quality in this case. + keepalive->gtFlags |= (GTF_CALL | GTF_GLOB_REF); + + return keepalive; +} + +inline GenTreeCast* Compiler::gtNewCastNode(var_types typ, GenTree* op1, bool fromUnsigned, var_types castType) +{ + GenTreeCast* cast = new (this, GT_CAST) GenTreeCast(typ, op1, fromUnsigned, castType); + + return cast; +} + +inline GenTreeCast* Compiler::gtNewCastNodeL(var_types typ, GenTree* op1, bool fromUnsigned, var_types castType) +{ + /* Some casts get transformed into 'GT_CALL' or 'GT_IND' nodes */ + + assert(GenTree::s_gtNodeSizes[GT_CALL] >= GenTree::s_gtNodeSizes[GT_CAST]); + assert(GenTree::s_gtNodeSizes[GT_CALL] >= GenTree::s_gtNodeSizes[GT_IND]); + + /* Make a big node first and then change it to be GT_CAST */ + + GenTreeCast* cast = + new (this, LargeOpOpcode()) GenTreeCast(typ, op1, fromUnsigned, castType DEBUGARG(/*largeNode*/ true)); + + return cast; +} + +inline GenTreeIndir* Compiler::gtNewMethodTableLookup(GenTree* object) +{ + assert(object->TypeIs(TYP_REF)); + GenTreeIndir* result = gtNewIndir(TYP_I_IMPL, object, GTF_IND_INVARIANT); + return result; +} + +inline void GenTree::SetOperRaw(genTreeOps oper) +{ + // Please do not do anything here other than assign to gtOper (debug-only + // code is OK, but should be kept to a minimum). + RecordOperBashing(OperGet(), oper); // nop unless NODEBASH_STATS is enabled + + // Bashing to MultiOp nodes is currently not supported. + assert(!OperIsMultiOp(oper)); + + gtOper = oper; +} + +//------------------------------------------------------------------------ +// SetOper: Bash this tree to an oper within its "class". +// +// "Bashing" refers to the act of mutating a node in-place to represent +// a different oper. It is effectively a custom version of the placement +// new operator. This method encapsulates the common logic for bashing +// nodes and is intended to be used when the new oper has the same "class" +// as that being bashed from. "Class" here is used somewhat loosely, but +// in general is tied to the GenTree subtype the new oper uses and what +// "namespace" of GenTreeFlags it belongs to - this method does not clear +// them. For example, GT_LCL_VAR and GT_LCL_FLD share the same "class", as +// do GT_IND and GT_BLK, etc. +// +// This method initializes some fields on a limited number of common tree +// subtypes, however, in general, it is the caller's responsibility to make +// sure the new node ends up in a valid state. +// +// Arguments: +// oper - The new oper +// vnUpdate - Whether to clear or preserve the VN (default: clear) +// inline void GenTree::SetOper(genTreeOps oper, ValueNumberUpdate vnUpdate) { assert(((gtDebugFlags & GTF_DEBUG_NODE_SMALL) != 0) != ((gtDebugFlags & GTF_DEBUG_NODE_LARGE) != 0)); @@ -1214,8 +1290,8 @@ inline void GenTree::SetOper(genTreeOps oper, ValueNumberUpdate vnUpdate) gtVNPair.SetBoth(ValueNumStore::NoVN); } - // Do "oper"-specific initializations. TODO-Cleanup: these are too ad-hoc to be reliable. - // The bashing code should decide itself what to initialize and what to leave as it was. + // Do some "oper"-specific initializations. These are not intended to be exhaustive but + // rather simplify calling code for common patterns. switch (oper) { case GT_CNS_INT: @@ -1253,71 +1329,16 @@ inline void GenTree::SetOper(genTreeOps oper, ValueNumberUpdate vnUpdate) } } -inline GenTree* Compiler::gtNewKeepAliveNode(GenTree* op) -{ - GenTree* keepalive = gtNewOperNode(GT_KEEPALIVE, TYP_VOID, op); - - // Prevent both reordering and removal. Invalid optimizations of GC.KeepAlive are - // very subtle and hard to observe. Thus we are conservatively marking it with both - // GTF_CALL and GTF_GLOB_REF side-effects even though it may be more than strictly - // necessary. The conservative side-effects are unlikely to have negative impact - // on code quality in this case. - keepalive->gtFlags |= (GTF_CALL | GTF_GLOB_REF); - - return keepalive; -} - -inline GenTreeCast* Compiler::gtNewCastNode(var_types typ, GenTree* op1, bool fromUnsigned, var_types castType) -{ - GenTreeCast* cast = new (this, GT_CAST) GenTreeCast(typ, op1, fromUnsigned, castType); - - return cast; -} - -inline GenTreeCast* Compiler::gtNewCastNodeL(var_types typ, GenTree* op1, bool fromUnsigned, var_types castType) -{ - /* Some casts get transformed into 'GT_CALL' or 'GT_IND' nodes */ - - assert(GenTree::s_gtNodeSizes[GT_CALL] >= GenTree::s_gtNodeSizes[GT_CAST]); - assert(GenTree::s_gtNodeSizes[GT_CALL] >= GenTree::s_gtNodeSizes[GT_IND]); - - /* Make a big node first and then change it to be GT_CAST */ - - GenTreeCast* cast = - new (this, LargeOpOpcode()) GenTreeCast(typ, op1, fromUnsigned, castType DEBUGARG(/*largeNode*/ true)); - - return cast; -} - -inline GenTreeIndir* Compiler::gtNewMethodTableLookup(GenTree* object) -{ - assert(object->TypeIs(TYP_REF)); - GenTreeIndir* result = gtNewIndir(TYP_I_IMPL, object, GTF_IND_INVARIANT); - return result; -} - -/*****************************************************************************/ - -/*****************************************************************************/ - -inline void GenTree::SetOperRaw(genTreeOps oper) -{ - // Please do not do anything here other than assign to gtOper (debug-only - // code is OK, but should be kept to a minimum). - RecordOperBashing(OperGet(), oper); // nop unless NODEBASH_STATS is enabled - - // Bashing to MultiOp nodes is not currently supported. - assert(!OperIsMultiOp(oper)); - - gtOper = oper; -} - -inline void GenTree::SetOperResetFlags(genTreeOps oper) -{ - SetOper(oper); - gtFlags &= GTF_NODE_MASK; -} - +//------------------------------------------------------------------------ +// ChangeOper: Bash this tree to an oper from a foreign "class". +// +// This bashing method, unlike "SetOper", clears oper-specific flags and +// so is more suitable to bashing nodes between different "classes". +// +// Arguments: +// oper - The new oper +// vnUpdate - Whether to clear or preserve the VN (default: clear) +// inline void GenTree::ChangeOper(genTreeOps oper, ValueNumberUpdate vnUpdate) { assert(!OperIsConst(oper)); // use BashToConst() instead @@ -1331,17 +1352,6 @@ inline void GenTree::ChangeOper(genTreeOps oper, ValueNumberUpdate vnUpdate) gtFlags &= mask; } -inline void GenTree::ChangeOperUnchecked(genTreeOps oper) -{ - GenTreeFlags mask = GTF_COMMON_MASK; - if (this->OperIsIndirOrArrMetaData() && OperIsIndirOrArrMetaData(oper)) - { - mask |= GTF_IND_NONFAULTING; - } - SetOperRaw(oper); // Trust the caller and don't use SetOper() - gtFlags &= mask; -} - //------------------------------------------------------------------------ // BashToConst: Bash the node to a constant one. // @@ -1396,7 +1406,8 @@ void GenTree::BashToConst(T value, var_types type /* = TYP_UNDEF */) oper = (type == TYP_LONG) ? GT_CNS_NATIVELONG : GT_CNS_INT; } - SetOperResetFlags(oper); + SetOper(oper); + gtFlags &= GTF_NODE_MASK; gtType = type; switch (oper) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 32013dc20c7..a88e3c89800 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -3040,8 +3040,8 @@ PhaseStatus Compiler::fgSimpleLowering() } // Change to a GT_IND. - tree->ChangeOperUnchecked(GT_IND); - tree->AsOp()->gtOp1 = addr; + tree->ChangeOper(GT_IND); + tree->AsIndir()->Addr() = addr; JITDUMP("After Lower %s:\n", GenTree::OpName(tree->OperGet())); DISPRANGE(LIR::ReadOnlyRange(arr, tree)); diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 80ebe022e66..1160027adf0 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1907,12 +1907,8 @@ public: PRESERVE_VN // Preserve value number }; - void SetOper(genTreeOps oper, ValueNumberUpdate vnUpdate = CLEAR_VN); // set gtOper - void SetOperResetFlags(genTreeOps oper); // set gtOper and reset flags - - // set gtOper and only keep GTF_COMMON_MASK flags + void SetOper(genTreeOps oper, ValueNumberUpdate vnUpdate = CLEAR_VN); void ChangeOper(genTreeOps oper, ValueNumberUpdate vnUpdate = CLEAR_VN); - void ChangeOperUnchecked(genTreeOps oper); void SetOperRaw(genTreeOps oper); void ChangeType(var_types newType)