[builtins] Prepare CSA::ConvertToRelativeIndex() for non-Smi indices

Bug: v8:4153
Change-Id: I88abd7b5b5ba1b7156d60427217fc8e301cbc099
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1866653
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64433}

src/builtins/base.tq

@@ -2035,6 +2035,7 @@ extern operator '==' macro Float64Equal(float64, float64): bool;
 extern operator '!=' macro Float64NotEqual(float64, float64): bool;
 extern operator '>' macro Float64GreaterThan(float64, float64): bool;
 extern operator '<' macro Float64LessThan(float64, float64): bool;
+extern operator '<=' macro Float64LessThanOrEqual(float64, float64): bool;
 
 extern macro BranchIfNumberEqual(Number, Number): never
     labels Taken, NotTaken;
@@ -2179,6 +2180,7 @@ extern macro SmiMod(Smi, Smi): Number;
 
 extern macro IntPtrMax(intptr, intptr): intptr;
 extern macro IntPtrMin(intptr, intptr): intptr;
+extern macro UintPtrMin(uintptr, uintptr): uintptr;
 
 extern operator '!' macro ConstexprBoolNot(constexpr bool): constexpr bool;
 extern operator '!' macro Word32BinaryNot(bool): bool;
@@ -3650,8 +3652,58 @@ extern macro NumberToString(Number): String;
 extern macro IsOneByteStringInstanceType(InstanceType): bool;
 extern macro AllocateSeqOneByteString(uint32): String;
 extern macro AllocateSeqTwoByteString(uint32): String;
-extern macro ConvertToRelativeIndex(implicit context: Context)(
-    JSAny, intptr): intptr;
+
+// After converting an index to an integer, calculate a relative index:
+// return index < 0 ? max(length + index, 0) : min(index, length)
+// TODO(v8:4153): make length and result uintptr.
+@export
+transitioning macro ConvertToRelativeIndex(implicit context: Context)(
+    index: JSAny, length: intptr): intptr {
+  const indexNumber: Number =
+      ToInteger_Inline(context, index, kTruncateMinusZero);
+  return ConvertToRelativeIndex(indexNumber, length);
+}
+
+// Calculate a relative index:
+// return index < 0 ? max(length + index, 0) : min(index, length)
+// TODO(v8:4153): make length and result uintptr.
+@export
+macro ConvertToRelativeIndex(indexNumber: Number, lengthIntPtr: intptr):
+    intptr {
+  const length: uintptr = Convert<uintptr>(lengthIntPtr);
+  typeswitch (indexNumber) {
+    case (indexSmi: Smi): {
+      const indexIntPtr: intptr = Convert<intptr>(indexSmi);
+      // The logic is implemented using unsigned types.
+      if (indexIntPtr < 0) {
+        const relativeIndex: uintptr = Unsigned(indexIntPtr) + length;
+        return Signed(relativeIndex < length ? relativeIndex : 0);
+
+      } else {
+        const relativeIndex: uintptr = Unsigned(indexIntPtr);
+        return Signed(relativeIndex < length ? relativeIndex : length);
+      }
+    }
+    case (indexHeapNumber: HeapNumber): {
+      const indexDouble: float64 = Convert<float64>(indexHeapNumber);
+      // NaNs must already be handled by ConvertToRelativeIndex() version
+      // above accepting JSAny indices.
+      assert(indexDouble == indexDouble);
+      const lengthDouble: float64 = Convert<float64>(length);
+      assert(lengthDouble <= kMaxSafeInteger);
+      if (indexDouble < 0) {
+        const relativeIndex: float64 = lengthDouble + indexDouble;
+        return relativeIndex > 0 ?
+            Signed(ChangeFloat64ToUintPtr(relativeIndex)) :
+            0;
+
+      } else {
+        return Signed(ChangeFloat64ToUintPtr(
+            indexDouble < lengthDouble ? indexDouble : lengthDouble));
+      }
+    }
+  }
+}
 
 extern builtin ObjectToString(Context, JSAny): JSAny;
 extern builtin StringRepeat(Context, String, Number): String;

src/builtins/string-slice.tq

@@ -19,13 +19,15 @@ namespace string_slice {
     const length: intptr = string.length_intptr;
 
     // Convert {start} to a relative index.
-    const start: intptr = ConvertToRelativeIndex(arguments[0], length);
+    const arg0 = arguments[0];
+    const start: intptr =
+        arg0 != Undefined ? ConvertToRelativeIndex(arg0, length) : 0;
 
     // 5. If end is undefined, let intEnd be len;
     // else Convert {end} to a relative index.
-    const temp = arguments[1];
+    const arg1 = arguments[1];
     const end: intptr =
-        temp == Undefined ? length : ConvertToRelativeIndex(temp, length);
+        arg1 != Undefined ? ConvertToRelativeIndex(arg1, length) : length;
 
     if (end <= start) {
       return kEmptyString;

src/codegen/code-stub-assembler.cc

@@ -927,44 +927,6 @@ TNode<Number> CodeStubAssembler::NumberMin(SloppyTNode<Number> a,
   return result.value();
 }
 
-TNode<IntPtrT> CodeStubAssembler::ConvertToRelativeIndex(
-    TNode<Context> context, TNode<Object> index, TNode<IntPtrT> length) {
-  TVARIABLE(IntPtrT, result);
-
-  TNode<Number> const index_int =
-      ToInteger_Inline(context, index, CodeStubAssembler::kTruncateMinusZero);
-  TNode<IntPtrT> zero = IntPtrConstant(0);
-
-  Label done(this);
-  Label if_issmi(this), if_isheapnumber(this, Label::kDeferred);
-  Branch(TaggedIsSmi(index_int), &if_issmi, &if_isheapnumber);
-
-  BIND(&if_issmi);
-  {
-    TNode<Smi> const index_smi = CAST(index_int);
-    result = Select<IntPtrT>(
-        IntPtrLessThan(SmiUntag(index_smi), zero),
-        [=] { return IntPtrMax(IntPtrAdd(length, SmiUntag(index_smi)), zero); },
-        [=] { return IntPtrMin(SmiUntag(index_smi), length); });
-    Goto(&done);
-  }
-
-  BIND(&if_isheapnumber);
-  {
-    // If {index} is a heap number, it is definitely out of bounds. If it is
-    // negative, {index} = max({length} + {index}),0) = 0'. If it is positive,
-    // set {index} to {length}.
-    TNode<HeapNumber> const index_hn = CAST(index_int);
-    TNode<Float64T> const float_zero = Float64Constant(0.);
-    TNode<Float64T> const index_float = LoadHeapNumberValue(index_hn);
-    result = SelectConstant<IntPtrT>(Float64LessThan(index_float, float_zero),
-                                     zero, length);
-    Goto(&done);
-  }
-  BIND(&done);
-  return result.value();
-}
-
 TNode<Number> CodeStubAssembler::SmiMod(TNode<Smi> a, TNode<Smi> b) {
   TVARIABLE(Number, var_result);
   Label return_result(this, &var_result),
@@ -5754,7 +5716,8 @@ TNode<UintPtrT> CodeStubAssembler::TryNumberToUintPtr(TNode<Number> value,
       [&] {
         TNode<Smi> value_smi = CAST(value);
         if (if_negative == nullptr) {
-          CSA_SLOW_ASSERT(this, SmiLessThan(SmiConstant(-1), value_smi));
+          CSA_SLOW_ASSERT(this,
+                          SmiGreaterThanOrEqual(value_smi, SmiConstant(0)));
         } else {
           GotoIfNot(TaggedIsPositiveSmi(value), if_negative);
         }
@@ -8024,6 +7987,19 @@ TNode<IntPtrT> CodeStubAssembler::IntPtrMin(SloppyTNode<IntPtrT> left,
                                  right);
 }
 
+TNode<UintPtrT> CodeStubAssembler::UintPtrMin(TNode<UintPtrT> left,
+                                              TNode<UintPtrT> right) {
+  intptr_t left_constant;
+  intptr_t right_constant;
+  if (ToIntPtrConstant(left, &left_constant) &&
+      ToIntPtrConstant(right, &right_constant)) {
+    return UintPtrConstant(std::min(static_cast<uintptr_t>(left_constant),
+                                    static_cast<uintptr_t>(right_constant)));
+  }
+  return SelectConstant<UintPtrT>(UintPtrLessThanOrEqual(left, right), left,
+                                  right);
+}
+
 template <>
 TNode<HeapObject> CodeStubAssembler::LoadName<NameDictionary>(
     TNode<HeapObject> key) {

src/codegen/code-stub-assembler.h

@@ -575,6 +575,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   // Select the minimum of the two provided IntPtr values.
   TNode<IntPtrT> IntPtrMin(SloppyTNode<IntPtrT> left,
                            SloppyTNode<IntPtrT> right);
+  TNode<UintPtrT> UintPtrMin(TNode<UintPtrT> left, TNode<UintPtrT> right);
 
   // Float64 operations.
   TNode<Float64T> Float64Ceil(SloppyTNode<Float64T> x);
@@ -587,12 +588,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   // Select the minimum of the two provided Number values.
   TNode<Number> NumberMin(SloppyTNode<Number> left, SloppyTNode<Number> right);
 
-  // After converting an index to an integer, calculate a relative index: if
-  // index < 0, max(length + index, 0); else min(index, length)
-  TNode<IntPtrT> ConvertToRelativeIndex(TNode<Context> context,
-                                        TNode<Object> index,
-                                        TNode<IntPtrT> length);
-
   // Returns true iff the given value fits into smi range and is >= 0.
   TNode<BoolT> IsValidPositiveSmi(TNode<IntPtrT> value);
 

test/cctest/test-code-stub-assembler.cc

@@ -325,6 +325,105 @@ TEST(IsValidPositiveSmi) {
 #endif
 }
 
+TEST(ConvertToRelativeIndex) {
+  Isolate* isolate(CcTest::InitIsolateOnce());
+
+  const int kNumParams = 3;
+  CodeAssemblerTester asm_tester(isolate, kNumParams);
+  CodeStubAssembler m(asm_tester.state());
+
+  enum Result { kFound, kNotFound };
+  {
+    TNode<Number> index = m.CAST(m.Parameter(0));
+    TNode<Number> length_number = m.CAST(m.Parameter(1));
+    TNode<Number> expected_relative_index = m.CAST(m.Parameter(2));
+
+    TNode<UintPtrT> length = m.ChangeNonnegativeNumberToUintPtr(length_number);
+    TNode<UintPtrT> expected =
+        m.ChangeNonnegativeNumberToUintPtr(expected_relative_index);
+
+    TNode<UintPtrT> result =
+        m.Unsigned(m.ConvertToRelativeIndex(index, m.Signed(length)));
+
+    m.Return(m.SelectBooleanConstant(m.WordEqual(result, expected)));
+  }
+
+  FunctionTester ft(asm_tester.GenerateCode(), kNumParams);
+
+  const double kMaxSmi = static_cast<double>(kSmiMaxValue);
+  const double kMaxInt32 =
+      static_cast<double>(std::numeric_limits<int32_t>::max());
+  const double kMaxUInt32 =
+      static_cast<double>(std::numeric_limits<uint32_t>::max());
+  const double kMaxUIntPtr =
+      static_cast<double>(std::numeric_limits<uintptr_t>::max());
+
+  struct {
+    double index;
+    double length;
+    double expected_result;
+  } test_cases[] = {
+      // Simple Smi-range cases.
+      {0, 0, 0},
+      {0, 42, 0},
+      {5, 42, 5},
+      {100, 42, 42},
+      {-10, 153, 153 - 10},
+      {-200, 153, 0},
+      // Beyond Smi-range index cases.
+      {0, kMaxSmi, 0},
+      {-153, kMaxSmi, kMaxSmi - 153},
+      {kMaxSmi + 153, kMaxSmi, kMaxSmi},
+      {kMaxSmi * 33, kMaxSmi, kMaxSmi},
+      {-kMaxSmi, kMaxSmi, 0},
+      {-kMaxSmi - 1, kMaxSmi, 0},
+      {-kMaxSmi - 153, kMaxSmi, 0},
+      {-kMaxSmi * 33, kMaxSmi, 0},
+      {-std::numeric_limits<double>::infinity(), 153, 0},
+      {std::numeric_limits<double>::infinity(), 424242, 424242},
+      // Beyond Smi-range length cases.
+      {kMaxSmi + 2, kMaxSmi + 1, kMaxSmi + 1},
+      {-kMaxSmi + 2, kMaxSmi + 1, 3},
+      {kMaxInt32 + 1, kMaxInt32, kMaxInt32},
+      {-kMaxInt32 + 1, kMaxInt32, 1},
+      {kMaxUInt32 + 1, kMaxUInt32, kMaxUInt32},
+      {-42, kMaxUInt32, kMaxUInt32 - 42},
+      {-kMaxUInt32 - 1, kMaxUInt32, 0},
+      {-kMaxUInt32, kMaxUInt32, 0},
+      {-kMaxUInt32 + 1, kMaxUInt32, 1},
+      {-kMaxUInt32 + 5, kMaxUInt32, 5},
+      {-kMaxUInt32 + 5, kMaxUInt32 + 1, 6},
+      {-kMaxSmi * 33, kMaxSmi * 153, kMaxSmi * (153 - 33)},
+      {0, kMaxSafeInteger, 0},
+      {kMaxSmi, kMaxSafeInteger, kMaxSmi},
+      {kMaxSmi * 153, kMaxSafeInteger, kMaxSmi * 153},
+      {-10, kMaxSafeInteger, kMaxSafeInteger - 10},
+      {-kMaxSafeInteger, kMaxSafeInteger, 0},
+      {-kMaxSafeInteger + 1, kMaxSafeInteger, 1},
+      {-kMaxSafeInteger + 42, kMaxSafeInteger, 42},
+      {kMaxSafeInteger - 153, kMaxSafeInteger, kMaxSafeInteger - 153},
+      {kMaxSafeInteger - 1, kMaxSafeInteger, kMaxSafeInteger - 1},
+      {kMaxSafeInteger, kMaxSafeInteger, kMaxSafeInteger},
+      {kMaxSafeInteger + 1, kMaxSafeInteger, kMaxSafeInteger},
+      {kMaxSafeInteger + 42, kMaxSafeInteger, kMaxSafeInteger},
+      {kMaxSafeInteger * 11, kMaxSafeInteger, kMaxSafeInteger},
+  };
+
+  Factory* factory = isolate->factory();
+  for (size_t i = 0; i < arraysize(test_cases); i++) {
+    if (test_cases[i].length > kMaxUIntPtr) {
+      // Test cases where length does not fit into uintptr are not valid, so
+      // skip them instead of ifdef'ing the test cases above.
+      continue;
+    }
+    Handle<Object> index = factory->NewNumber(test_cases[i].index);
+    Handle<Object> length = factory->NewNumber(test_cases[i].length);
+    Handle<Object> expected = factory->NewNumber(test_cases[i].expected_result);
+
+    ft.CheckTrue(index, length, expected);
+  }
+}
+
 TEST(FixedArrayAccessSmiIndex) {
   Isolate* isolate(CcTest::InitIsolateOnce());
   CodeAssemblerTester asm_tester(isolate);