Fix simple general warnings found by PVS. (#27283)

* Delete unused `scAvailable`.

* Delete unused `PhysRegIntervalIterator`.

* Expression is always true.

* Possible overflow.

* Delete unreachable/unused code.

* Two or more case-branches perform the same actions.

* Check self-copying.

* The 'isOnStack' variable is assigned values twice successively.

* The 'then' statement is equivalent to the 'else' statement.

`MAX_COST` is `UCHAR_MAX` so if `unsigned char cost >= MAX_COST` then it is equal to ``MAX_COST`.
So the code was correct, but confusing.

* Expression 'newPage == nullptr' is always true.

* Expression 'inconsistentProfileData' is always false.

* A part of conditional expression is always false: typ == TYP_BOOL.

* A part of conditional expression is always true: emitFullGCinfo.

We have an early return from this function if `emitFullGCinfo == false`.

* A part of conditional expression is always true: asgNode.

We have an early return if it is nullptr.

* delete an unused function with errors.

There were 3 incorrent printf arguments.

* Delete another unused function without return.

* An excessive check can be simplified. The '(A && B) || (!A && !B)' expression is equivalent to the 'bool(A) == bool(B)' expression,

* The expression is of enum type. It is odd that it is used as an expression of a Boolean-type.

* the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead.

That function and almost the whole file is unused, do we want to delete it?

* The expression 'eeGetHelperNum(method)' is of enum type. It is odd that it is used as an expression of a Boolean-type.

* `nodeThis` and `nodeOther` are always true.

* 'this == nullptr' expression should be avoided - this expression is always false on newer compilers, because 'this' pointer can never be NULL.

Don't compare `this` with null.

* Variables 'm_Cost', 'm_Size' are initialized through the call to the same function. It's probably an error or un-optimized code. Consider inspecting the 'Expr()->GetCostSz()' expression.

* Parameter 'toRefPosition' is not used inside function body.

* Expression 'parentOfArgObj == parentArgx' is always true.

We assigned `parentOfArgObj ` to `parentArgx` and did not change any of them before that check.

* The 'srcCount' variable is assigned values twice successively. Perhaps this is a mistake.

* Response review.

Author: Sergey Andreenko

src/inc/safemath.h

@@ -524,6 +524,7 @@ template<typename T> class ClrSafeInt
                         }
 
 #ifdef _MSC_VER
+#pragma warning(push)
 #pragma warning( disable : 4146 )   // unary minus applied to unsigned is still unsigned
 #endif
                         if(MaxInt()/(-lhs) < (-rhs) )
@@ -532,7 +533,7 @@ template<typename T> class ClrSafeInt
                             return false;
                         }
 #ifdef _MSC_VER
-#pragma warning( default : 4146 )
+#pragma warning(pop)
 #endif
                     }
                 }

src/jit/alloc.cpp

@@ -81,23 +81,18 @@ void* ArenaAllocator::allocateNewPage(size_t size)
         m_lastPage->m_usedBytes = m_nextFreeByte - m_lastPage->m_contents;
     }
 
-    PageDescriptor* newPage = nullptr;
-
     if (!bypassHostAllocator())
     {
         // Round to the nearest multiple of default page size
         pageSize = roundUp(pageSize, DEFAULT_PAGE_SIZE);
     }
 
+    // Allocate the new page
+    PageDescriptor* newPage = static_cast<PageDescriptor*>(allocateHostMemory(pageSize, &pageSize));
+
     if (newPage == nullptr)
     {
-        // Allocate the new page
-        newPage = static_cast<PageDescriptor*>(allocateHostMemory(pageSize, &pageSize));
-
-        if (newPage == nullptr)
-        {
-            NOMEM();
-        }
+        NOMEM();
     }
 
     // Append the new page to the end of the list

src/jit/assertionprop.cpp

@@ -85,7 +85,7 @@ void Compiler::optAddCopies()
            tracks simple assignments. The same goes for lvNormalizedOnStore as
            the cast is generated in fgMorphSmpOpAsg. This boils down to not having
            a copy until optAssertionGen handles this*/
-        if (varDsc->lvNormalizeOnLoad() || varDsc->lvNormalizeOnStore() || typ == TYP_BOOL)
+        if (varDsc->lvNormalizeOnLoad() || varDsc->lvNormalizeOnStore())
         {
             continue;
         }

src/jit/codegen.h

@@ -647,7 +647,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
         unsigned scLVnum;  // 'which' in eeGetLVinfo()
 
         unsigned scStackLevel; // Only for stk-vars
-        bool scAvailable : 1;  // It has a home / Home recycled - TODO-Cleanup: it appears this is unused (always true)
 
         siScope* scPrev;
         siScope* scNext;

src/jit/codegencommon.cpp

@@ -10183,8 +10183,8 @@ void CodeGen::genSetScopeInfoUsingsiScope()
         LclVarDsc* varDsc = compiler->lvaGetDesc(scopeL->scVarNum);
         siVarLoc   varLoc = getSiVarLoc(varDsc, scopeL);
 
-        genSetScopeInfo(psiScopeCnt + i, startOffs, endOffs - startOffs, scopeL->scVarNum, scopeL->scLVnum,
-                        scopeL->scAvailable, &varLoc);
+        genSetScopeInfo(psiScopeCnt + i, startOffs, endOffs - startOffs, scopeL->scVarNum, scopeL->scLVnum, false,
+                        &varLoc);
     }
 }
 #endif // USING_SCOPE_INFO

src/jit/compiler.h

@@ -8092,15 +8092,11 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
     {
 #ifdef FEATURE_SIMD
         unsigned vectorRegSize = getSIMDVectorRegisterByteLength();
-        if (vectorRegSize > TARGET_POINTER_SIZE)
-        {
-            return vectorRegSize;
-        }
-        else
-#endif // FEATURE_SIMD
-        {
-            return TARGET_POINTER_SIZE;
-        }
+        assert(vectorRegSize >= TARGET_POINTER_SIZE);
+        return vectorRegSize;
+#else  // !FEATURE_SIMD
+        return TARGET_POINTER_SIZE;
+#endif // !FEATURE_SIMD
     }
 
 private:

src/jit/compiler.hpp

@@ -3592,7 +3592,7 @@ inline CORINFO_METHOD_HANDLE Compiler::eeFindHelper(unsigned helper)
     /* Helpers are marked by the fact that they are odd numbers
      * force this to be an odd number (will shift it back to extract) */
 
-    return ((CORINFO_METHOD_HANDLE)(size_t)((helper << 2) + 1));
+    return ((CORINFO_METHOD_HANDLE)((((size_t)helper) << 2) + 1));
 }
 
 inline CorInfoHelpFunc Compiler::eeGetHelperNum(CORINFO_METHOD_HANDLE method)

src/jit/ee_il_dll.cpp

@@ -1308,7 +1308,7 @@ static LONG FilterSuperPMIExceptions_ee_il(PEXCEPTION_POINTERS pExceptionPointer
 
 const char* Compiler::eeGetMethodName(CORINFO_METHOD_HANDLE method, const char** classNamePtr)
 {
-    if (eeGetHelperNum(method))
+    if (eeGetHelperNum(method) != CORINFO_HELP_UNDEF)
     {
         if (classNamePtr != nullptr)
         {

src/jit/emit.cpp

@@ -7375,7 +7375,7 @@ void emitter::emitStackKillArgs(BYTE* addr, unsigned count, unsigned char callIn
            stack, but they are effectively dead. For fully-interruptible
            methods, we need to report that */
 
-        if (emitFullGCinfo && gcCnt.Value())
+        if (gcCnt.Value())
         {
             /* Allocate a new ptr arg entry and fill it in */
 

src/jit/flowgraph.cpp

@@ -10433,10 +10433,8 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
             block->bbWeight = bNext->bbWeight;
 
             block->bbFlags |= (bNext->bbFlags & BBF_PROF_WEIGHT); // Set the profile weight flag (if necessary)
-            if (block->bbWeight != 0)
-            {
-                block->bbFlags &= ~BBF_RUN_RARELY; // Clear any RarelyRun flag
-            }
+            assert(block->bbWeight != 0);
+            block->bbFlags &= ~BBF_RUN_RARELY; // Clear any RarelyRun flag
         }
     }
     // otherwise if either block has a zero weight we select the zero weight
@@ -13652,11 +13650,7 @@ void Compiler::fgComputeEdgeWeights()
             }
         }
 
-        if (inconsistentProfileData)
-        {
-            hasIncompleteEdgeWeights = true;
-            break;
-        }
+        assert(!inconsistentProfileData); // Should use EARLY_EXIT when it is false.
 
         if (numEdges == goodEdgeCountCurrent)
         {
@@ -15011,8 +15005,7 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
         }
         else
         {
-            BasicBlock::weight_t newWeightDest    = 0;
-            BasicBlock::weight_t unloopWeightDest = 0;
+            BasicBlock::weight_t newWeightDest = 0;
 
             if (weightDest > weightJump)
             {
@@ -15022,9 +15015,9 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
             {
                 newWeightDest = (weightDest * 2) / (BB_LOOP_WEIGHT * BB_UNITY_WEIGHT);
             }
-            if ((newWeightDest > 0) || (unloopWeightDest > 0))
+            if (newWeightDest > 0)
             {
-                bDest->bbWeight = Max(newWeightDest, unloopWeightDest);
+                bDest->bbWeight = newWeightDest;
             }
         }
     }
@@ -19240,16 +19233,7 @@ unsigned Compiler::fgGetCodeEstimate(BasicBlock* block)
     for (Statement* stmt : StatementList(block->FirstNonPhiDef()))
     {
         unsigned char cost = stmt->GetCostSz();
-        if (cost < MAX_COST)
-        {
-            costSz += cost;
-        }
-        else
-        {
-            // We could walk the tree to find out the real GetCostSz(),
-            // but just using MAX_COST for this trees code size works OK
-            costSz += cost;
-        }
+        costSz += cost;
     }
 
     return costSz;