Tweak Regex comparison to improve subsequent bounds check removals (d…

…otnet#68846) * Tweak Regex comparison to improve subsequent bounds check removals * Address PR feedback
radical · May 5, 2022 · 393be55 · 393be55
1 parent 8a31a32
commit 393be55
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 14 deletions.
diff --git a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
@@ -824,12 +824,26 @@ void EmitFixedSet_LeftToRight()
 
                         if (setsToUse > 1)
                         {
-                            using (EmitBlock(writer, $"if (i >= span.Length - {minRequiredLength - 1})"))
+                            // Of the remaining sets we're going to check, find the maximum distance of any of them.
+                            // If it's further than the primary set we checked, we need a bounds check.
+                            int maxDistance = sets[1].Distance;
+                            for (int i = 2; i < setsToUse; i++)
                             {
-                                noMatchFoundLabelNeeded = true;
-                                Goto(NoMatchFound);
+                                maxDistance = Math.Max(maxDistance, sets[i].Distance);
+                            }
+                            if (maxDistance > primarySet.Distance)
+                            {
+                                int numRemainingSets = setsToUse - 1;
+                                writer.WriteLine($"// The primary set being searched for was found. {numRemainingSets} more set{(numRemainingSets > 1 ? "s" : "")} will be checked so as");
+                                writer.WriteLine($"// to minimize the number of places TryMatchAtCurrentPosition is run unnecessarily.");
+                                writer.WriteLine($"// Make sure {(numRemainingSets > 1 ? "they fit" : "it fits")} in the remainder of the input.");
+                                using (EmitBlock(writer, $"if ((uint)(i + {maxDistance}) >= span.Length)"))
+                                {
+                                    noMatchFoundLabelNeeded = true;
+                                    Goto(NoMatchFound);
+                                }
+                                writer.WriteLine();
                             }
-                            writer.WriteLine();
                         }
                     }
                     else

diff --git a/...raries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexCompiler.cs b/...raries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexCompiler.cs
@@ -898,16 +898,29 @@ void EmitFixedSet_LeftToRight()
                         BltFar(returnFalse);
                     }
 
-                    // if (i >= slice.Length - (minRequiredLength - 1)) goto returnFalse;
-                    if (sets.Count > 1)
+                    if (setsToUse > 1)
                     {
-                        Debug.Assert(needLoop);
-                        Ldloca(textSpanLocal);
-                        Call(s_spanGetLengthMethod);
-                        Ldc(minRequiredLength - 1);
-                        Sub();
-                        Ldloc(iLocal);
-                        BleFar(returnFalse);
+                        // Of the remaining sets we're going to check, find the maximum distance of any of them.
+                        // If it's further than the primary set we checked, we need a bounds check.
+                        int maxDistance = sets[1].Distance;
+                        for (int i = 2; i < setsToUse; i++)
+                        {
+                            maxDistance = Math.Max(maxDistance, sets[i].Distance);
+                        }
+                        if (maxDistance > primarySet.Distance)
+                        {
+                            // if ((uint)(i + maxDistance) >= slice.Length) goto returnFalse;
+                            if (setsToUse > 1)
+                            {
+                                Debug.Assert(needLoop);
+                                Ldloc(iLocal);
+                                Ldc(maxDistance);
+                                Add();
+                                Ldloca(textSpanLocal);
+                                Call(s_spanGetLengthMethod);
+                                _ilg!.Emit(OpCodes.Bge_Un, returnFalse);
+                            }
+                        }
                     }
                 }
 
@@ -916,7 +929,7 @@ void EmitFixedSet_LeftToRight()
                 // if (!CharInClass(slice[i + 2], prefix[2], "...")) continue;
                 // ...
                 Debug.Assert(setIndex is 0 or 1);
-                for ( ; setIndex < sets.Count; setIndex++)
+                for ( ; setIndex < setsToUse; setIndex++)
                 {
                     Debug.Assert(needLoop);
                     Ldloca(textSpanLocal);