Improve perf for Index based span indexers (#21196)

First, evaluate the actual index before invoking the underlying int indexer,
so that the jit doesn't think the span is address taken.

Second, improve the jit's ability to remove some redundant comparisons like
those that arise in computing the actual index.

diff --git a/src/System.Private.CoreLib/shared/System/ReadOnlySpan.Fast.cs b/src/System.Private.CoreLib/shared/System/ReadOnlySpan.Fast.cs
index 2d04fd1006e11beb086f033813998d008d39a1a5..a24aaa94ee4543afd6e0222490e2e57a64263b2a 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/ReadOnlySpan.Fast.cs
+++ b/src/System.Private.CoreLib/shared/System/ReadOnlySpan.Fast.cs
@@ -158,7 +158,9 @@ namespace System
         {
             get
             {
-                return ref this [index.FromEnd ? _length - index.Value : index.Value];
+                // Evaluate the actual index first because it helps performance
+                int actualIndex = index.FromEnd ? _length - index.Value : index.Value;
+                return ref this [actualIndex];
             }
         }
 

diff --git a/src/System.Private.CoreLib/shared/System/Span.Fast.cs b/src/System.Private.CoreLib/shared/System/Span.Fast.cs
index b6e6253117e484b2de212b7e89d110e4c7fb7411..526987c115e5f0f414996caee7623acddfc2bacd 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Span.Fast.cs
+++ b/src/System.Private.CoreLib/shared/System/Span.Fast.cs
@@ -163,7 +163,9 @@ namespace System
         {
             get
             {
-                return ref this [index.FromEnd ? _length - index.Value : index.Value];
+                // Evaluate the actual index first because it helps performance
+                int actualIndex = index.FromEnd ? _length - index.Value : index.Value;
+                return ref this [actualIndex];
             }
         }
 

diff --git a/src/jit/assertionprop.cpp b/src/jit/assertionprop.cpp
index dffc5b999fc1765f00ee72135e57eec870a24a93..91892b4a5db5b474b4ee40095e96ce84aa76cf57 100644 (file)
--- a/src/jit/assertionprop.cpp
+++ b/src/jit/assertionprop.cpp
@@ -631,7 +631,7 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse
     }
     else if (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND)
     {
-        printf("Loop_Bnd");
+        printf("Const_Loop_Bnd");
         vnStore->vnDump(this, curAssertion->op1.vn);
     }
     else if (curAssertion->op1.kind == O1K_VALUE_NUMBER)
@@ -2996,6 +2996,42 @@ AssertionIndex Compiler::optGlobalAssertionIsEqualOrNotEqual(ASSERT_VALARG_TP as
     return NO_ASSERTION_INDEX;
 }
 
+/*****************************************************************************
+ *
+ *  Given a set of "assertions" to search for, find an assertion that is either
+ *  op == 0 or op != 0
+ *
+ */
+AssertionIndex Compiler::optGlobalAssertionIsEqualOrNotEqualZero(ASSERT_VALARG_TP assertions, GenTree* op1)
+{
+    if (BitVecOps::IsEmpty(apTraits, assertions))
+    {
+        return NO_ASSERTION_INDEX;
+    }
+    BitVecOps::Iter iter(apTraits, assertions);
+    unsigned        index = 0;
+    while (iter.NextElem(&index))
+    {
+        AssertionIndex assertionIndex = GetAssertionIndex(index);
+        if (assertionIndex > optAssertionCount)
+        {
+            break;
+        }
+        AssertionDsc* curAssertion = optGetAssertion(assertionIndex);
+        if ((curAssertion->assertionKind != OAK_EQUAL && curAssertion->assertionKind != OAK_NOT_EQUAL))
+        {
+            continue;
+        }
+
+        if ((curAssertion->op1.vn == vnStore->VNConservativeNormalValue(op1->gtVNPair)) &&
+            (curAssertion->op2.vn == vnStore->VNZeroForType(op1->TypeGet())))
+        {
+            return assertionIndex;
+        }
+    }
+    return NO_ASSERTION_INDEX;
+}
+
 /*****************************************************************************
  *
  *  Given a tree consisting of a RelOp and a set of available assertions
@@ -3008,24 +3044,23 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree*
 {
     assert(tree->OperKind() & GTK_RELOP);
 
-    //
-    // Currently only GT_EQ or GT_NE are supported Relops for AssertionProp
-    //
-    if ((tree->gtOper != GT_EQ) && (tree->gtOper != GT_NE))
-    {
-        return nullptr;
-    }
-
     if (!optLocalAssertionProp)
     {
         // If global assertion prop then use value numbering.
         return optAssertionPropGlobal_RelOp(assertions, tree, stmt);
     }
-    else
+
+    //
+    // Currently only GT_EQ or GT_NE are supported Relops for local AssertionProp
+    //
+
+    if ((tree->gtOper != GT_EQ) && (tree->gtOper != GT_NE))
     {
-        // If local assertion prop then use variable based prop.
-        return optAssertionPropLocal_RelOp(assertions, tree, stmt);
+        return nullptr;
     }
+
+    // If local assertion prop then use variable based prop.
+    return optAssertionPropLocal_RelOp(assertions, tree, stmt);
 }
 
 /*************************************************************************************
@@ -3036,19 +3071,65 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree*
  */
 GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, GenTree* stmt)
 {
-    assert(tree->OperGet() == GT_EQ || tree->OperGet() == GT_NE);
-
     GenTree* newTree = tree;
     GenTree* op1     = tree->gtOp.gtOp1;
     GenTree* op2     = tree->gtOp.gtOp2;
 
+    // Look for assertions of the form (tree EQ/NE 0)
+    AssertionIndex index = optGlobalAssertionIsEqualOrNotEqualZero(assertions, tree);
+
+    if (index != NO_ASSERTION_INDEX)
+    {
+        // We know that this relop is either 0 or != 0 (1)
+        AssertionDsc* curAssertion = optGetAssertion(index);
+
+#ifdef DEBUG
+        if (verbose)
+        {
+            printf("\nVN relop based constant assertion prop in " FMT_BB ":\n", compCurBB->bbNum);
+            printf("Assertion index=#%02u: ", index);
+            printTreeID(tree);
+            printf(" %s 0\n", (curAssertion->assertionKind == OAK_EQUAL) ? "==" : "!=");
+        }
+#endif
+
+        // Bail out if tree is not side effect free.
+        if ((tree->gtFlags & GTF_SIDE_EFFECT) != 0)
+        {
+            JITDUMP("sorry, blocked by side effects\n");
+            return nullptr;
+        }
+
+        if (curAssertion->assertionKind == OAK_EQUAL)
+        {
+            tree->ChangeOperConst(GT_CNS_INT);
+            tree->gtIntCon.gtIconVal = 0;
+        }
+        else
+        {
+            tree->ChangeOperConst(GT_CNS_INT);
+            tree->gtIntCon.gtIconVal = 1;
+        }
+
+        newTree = fgMorphTree(tree);
+        DISPTREE(newTree);
+        return optAssertionProp_Update(newTree, tree, stmt);
+    }
+
+    // Else check if we have an equality check involving a local
+    if (!tree->OperIs(GT_EQ, GT_NE))
+    {
+        return nullptr;
+    }
+
     if (op1->gtOper != GT_LCL_VAR)
     {
         return nullptr;
     }
 
     // Find an equal or not equal assertion involving "op1" and "op2".
-    AssertionIndex index = optGlobalAssertionIsEqualOrNotEqual(assertions, op1, op2);
+    index = optGlobalAssertionIsEqualOrNotEqual(assertions, op1, op2);
+
     if (index == NO_ASSERTION_INDEX)
     {
         return nullptr;

diff --git a/src/jit/compiler.h b/src/jit/compiler.h
index 1e5663ff0b07dbf5bd1ede686f17a602af29a652..b973e111054f58202a9cbb8cf92a0184bffdc407 100644 (file)
--- a/src/jit/compiler.h
+++ b/src/jit/compiler.h
@@ -6518,6 +6518,7 @@ public:
 
     // Used for Relop propagation.
     AssertionIndex optGlobalAssertionIsEqualOrNotEqual(ASSERT_VALARG_TP assertions, GenTree* op1, GenTree* op2);
+    AssertionIndex optGlobalAssertionIsEqualOrNotEqualZero(ASSERT_VALARG_TP assertions, GenTree* op1);
     AssertionIndex optLocalAssertionIsEqualOrNotEqual(
         optOp1Kind op1Kind, unsigned lclNum, optOp2Kind op2Kind, ssize_t cnsVal, ASSERT_VALARG_TP assertions);