WIP - omit rangechecks for no. prefixes

WIP - Attempting to remove array bound checks in certain cases when `no. rangecheck` is specified. Part of fix to #17469
john-h-k · Apr 9, 2019 · 9db3656 · 9db3656
1 parent 353b58c
commit 9db3656
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 10 deletions.
diff --git a/src/inc/opcode.def b/src/inc/opcode.def
@@ -316,7 +316,7 @@ OPDEF(CEE_INITOBJ,                    "initobj",          PopI,               Pu
 OPDEF(CEE_CONSTRAINED,                "constrained.",     Pop0,               Push0,       InlineType,         IPrefix,     2,  0xFE,    0x16,    META)
 OPDEF(CEE_CPBLK,                      "cpblk",            PopI+PopI+PopI,     Push0,       InlineNone,         IPrimitive,  2,  0xFE,    0x17,    NEXT)
 OPDEF(CEE_INITBLK,                    "initblk",          PopI+PopI+PopI,     Push0,       InlineNone,         IPrimitive,  2,  0xFE,    0x18,    NEXT)
-OPDEF(CEE_NO,                         "no.",              Pop0,               Push0,       ShortInlineI,         IPrefix,     2,  0xFE,    0x19,    META)
+OPDEF(CEE_NO,                         "no.",              Pop0,               Push0,       ShortInlineI,       IPrefix,     2,  0xFE,    0x19,    META)
 OPDEF(CEE_RETHROW,                    "rethrow",          Pop0,               Push0,       InlineNone,         IObjModel,   2,  0xFE,    0x1A,    THROW)
 OPDEF(CEE_UNUSED51,                   "unused",           Pop0,               Push0,       InlineNone,         IPrimitive,  2,  0xFE,    0x1B,    NEXT)
 OPDEF(CEE_SIZEOF,                     "sizeof",           Pop0,               PushI,       InlineType,         IPrimitive,  2,  0xFE,    0x1C,    NEXT)

diff --git a/src/jit/compiler.h b/src/jit/compiler.h
@@ -2634,7 +2634,7 @@ class Compiler
 
     GenTree* gtNewFieldRef(var_types typ, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj = nullptr, DWORD offset = 0);
 
-    GenTree* gtNewIndexRef(var_types typ, GenTree* arrayOp, GenTree* indexOp);
+    GenTree* gtNewIndexRef(var_types typ, GenTree* arrayOp, GenTree* indexOp, bool rngChkRequired = true);
 
     GenTreeArrLen* gtNewArrLen(var_types typ, GenTree* arrayOp, int lenOffset);
 

diff --git a/src/jit/compiler.hpp b/src/jit/compiler.hpp
@@ -1208,10 +1208,17 @@ inline GenTree* Compiler::gtNewFieldRef(var_types typ, CORINFO_FIELD_HANDLE fldH
  *  A little helper to create an array index node.
  */
 
-inline GenTree* Compiler::gtNewIndexRef(var_types typ, GenTree* arrayOp, GenTree* indexOp)
+inline GenTree* Compiler::gtNewIndexRef(var_types typ, GenTree* arrayOp, GenTree* indexOp, bool rngChkRequired)
 {
     GenTreeIndex* gtIndx = new (this, GT_INDEX) GenTreeIndex(typ, arrayOp, indexOp, genTypeSize(typ));
 
+    assert(!(gtIndx->gtFlags & GTF_INX_RNGCHK));
+
+    if (rngChkRequired)
+    {
+        gtIndx->gtFlags |= GTF_INX_RNGCHK;
+    }
+
     return gtIndx;
 }
 

diff --git a/src/jit/gentree.h b/src/jit/gentree.h
@@ -4139,6 +4139,7 @@ struct GenTreeIndex : public GenTreeOp
         }
         else
 #endif
+        if (arr->gtFlags & GTF_INX_RNGCHK)
         {
             // Do bounds check
             gtFlags |= GTF_INX_RNGCHK;

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
@@ -7009,7 +7009,7 @@ enum
 {
     NO_PREFIX_TYPECHECK = 0x01,
     NO_PREFIX_RANGECHECK = 0x02,
-    NO_PREFIX_NULLCHECK = 0x04
+    NO_PREFIX_NULLCHECK = 0x04,
 };
 
 // For prefixFlags
@@ -7023,7 +7023,11 @@ enum
     PREFIX_UNALIGNED   = 0x00001000,
     PREFIX_CONSTRAINED = 0x00010000,
     PREFIX_READONLY    = 0x00100000,
-    PREFIX_NO          = 0x01000000,
+    PREFIX_NO_TYPECHK  = 0x01000000,
+    PREFIX_NO_RANGECHK = 0x10000000,
+    PREFIX_NO_NULLCHK  = 0x00000002, // irregular value, an issue?
+    PREFIX_NO_ANY      = PREFIX_NO_TYPECHK | PREFIX_NO_RANGECHK | PREFIX_NO_NULLCHK
+
 };
 
 /********************************************************************************
@@ -11671,7 +11675,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
 
                 /* Create the index node and push it on the stack */
 
-                op1 = gtNewIndexRef(lclTyp, op1, op2);
+                op1 = gtNewIndexRef(lclTyp, op3, op1, prefixFlags & PREFIX_NO_RANGECHK);
 
                 ldstruct = (opcode == CEE_LDELEM && lclTyp == TYP_STRUCT);
 
@@ -11904,7 +11908,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
 
                 /* Create the index node */
 
-                op1 = gtNewIndexRef(lclTyp, op3, op1);
+                op1 = gtNewIndexRef(lclTyp, op3, op1, prefixFlags & PREFIX_NO_RANGECHK);
 
                 /* Create the assignment node and append it */
 
@@ -13267,12 +13271,23 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                 // should zero be allowed to indicate no checks omitted? (in this impl it is)
                 // should we enforce only relevant bits set? (in this impl it is not - 0b_1111_1111 is valid)
 
-                Verify(!(prefixFlags & PREFIX_NO), "Multiple .no prefixes");
+                Verify(!(prefixFlags & PREFIX_NO_ANY), "Multiple no. prefixes with equal values");
+
+                if (val & NO_PREFIX_TYPECHECK)
+                {
+                    prefixFlags |= PREFIX_NO_TYPECHK;
+                }
+                if (val & NO_PREFIX_RANGECHECK)
+                {
+                    prefixFlags |= PREFIX_NO_RANGECHK;
+                }
+                if (val & NO_PREFIX_NULLCHECK)
+                {
+                    prefixFlags |= PREFIX_NO_NULLCHK;
+                }
 
                 impValidateCheckOmittionOpcode(codeAddr, codeEndp, val);
 
-                prefixFlags |= PREFIX_NO; // will this cause any issue? it is useful to prevent multiple prefixes
-
                 assert(sz == 1);
 
                 goto PREFIX;
-Original file line number
+Diff line change
@@ Expand Up / @@ -4139,6 +4139,7 @@ struct GenTreeIndex : public GenTreeOp @@
             }
             else
     #endif
+            if (arr->gtFlags & GTF_INX_RNGCHK)
             {
                 // Do bounds check
                 gtFlags |= GTF_INX_RNGCHK;
@@ Expand Down @@