[cleanup] Improve types in codestub assembler

This changes the return type of
  - StringBuiltinsAssembler::LoadSurrogatePairAt
  - CodeStubAssembler::StringCharCodeAt
from TNode<Uint32T> to TNode<Int32T>.

This is justified because both functions only
return values in the positive range of signed
integer. This improves interoperatability, as
Int32T can be SmiTagged, while this is not
allowed for Uint32T.

Bug: v8:7270

Change-Id: I2768b6ec320fa0fbcf3e55af784339472fa4909e
Reviewed-on: https://chromium-review.googlesource.com/861782Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50542}

src/builtins/builtins-string-gen.cc

@@ -531,10 +531,10 @@ TF_BUILTIN(StringCharAt, CodeStubAssembler) {
   Node* position = Parameter(Descriptor::kPosition);
 
   // Load the character code at the {position} from the {receiver}.
-  Node* code = StringCharCodeAt(receiver, position);
+  TNode<Int32T> code = StringCharCodeAt(receiver, position);
 
   // And return the single character string with only that {code}
-  Node* result = StringFromCharCode(code);
+  TNode<String> result = StringFromCharCode(code);
   Return(result);
 }
 
@@ -543,11 +543,11 @@ TF_BUILTIN(StringCharCodeAt, CodeStubAssembler) {
   Node* position = Parameter(Descriptor::kPosition);
 
   // Load the character code at the {position} from the {receiver}.
-  Node* code = StringCharCodeAt(receiver, position);
+  TNode<Int32T> code = StringCharCodeAt(receiver, position);
 
   // And return it as TaggedSigned value.
   // TODO(turbofan): Allow builtins to return values untagged.
-  Node* result = SmiFromWord32(code);
+  TNode<Smi> result = SmiFromWord32(code);
   Return(result);
 }
 
@@ -577,7 +577,8 @@ TF_BUILTIN(StringFromCharCode, CodeStubAssembler) {
     // string on the fly otherwise.
     Node* code = arguments.AtIndex(0);
     Node* code32 = TruncateTaggedToWord32(context, code);
-    Node* code16 = Word32And(code32, Int32Constant(String::kMaxUtf16CodeUnit));
+    TNode<Int32T> code16 =
+        Signed(Word32And(code32, Int32Constant(String::kMaxUtf16CodeUnit)));
     Node* result = StringFromCharCode(code16);
     arguments.PopAndReturn(result);
   }
@@ -699,7 +700,7 @@ TF_BUILTIN(StringPrototypeCharAt, CodeStubAssembler) {
                                   LoadStringLengthAsWord(receiver)));
   CSA_ASSERT(this, IntPtrGreaterThanOrEqual(SmiUntag(position_smi),
                                             IntPtrConstant(0)));
-  Node* code = StringCharCodeAt(receiver, SmiUntag(position_smi));
+  TNode<Int32T> code = StringCharCodeAt(receiver, SmiUntag(position_smi));
 
   // And return the single character string with only that {code}.
   Node* result = StringFromCharCode(code);
@@ -2301,14 +2302,14 @@ TF_BUILTIN(StringPrototypeIterator, CodeStubAssembler) {
 
 // Return the |word32| codepoint at {index}. Supports SeqStrings and
 // ExternalStrings.
-TNode<Uint32T> StringBuiltinsAssembler::LoadSurrogatePairAt(
+TNode<Int32T> StringBuiltinsAssembler::LoadSurrogatePairAt(
     SloppyTNode<String> string, SloppyTNode<IntPtrT> length,
     SloppyTNode<IntPtrT> index, UnicodeEncoding encoding) {
   Label handle_surrogate_pair(this), return_result(this);
-  TVARIABLE(Uint32T, var_result);
-  TVARIABLE(Uint32T, var_trail);
+  TVARIABLE(Int32T, var_result);
+  TVARIABLE(Int32T, var_trail);
   var_result = StringCharCodeAt(string, index);
-  var_trail = Unsigned(Int32Constant(0));
+  var_trail = Int32Constant(0);
 
   GotoIf(Word32NotEqual(Word32And(var_result, Int32Constant(0xFC00)),
                         Int32Constant(0xD800)),
@@ -2323,8 +2324,8 @@ TNode<Uint32T> StringBuiltinsAssembler::LoadSurrogatePairAt(
 
   BIND(&handle_surrogate_pair);
   {
-    TNode<Uint32T> lead = var_result;
-    TNode<Uint32T> trail = var_trail;
+    TNode<Int32T> lead = var_result;
+    TNode<Int32T> trail = var_trail;
 
     // Check that this path is only taken if a surrogate pair is found
     CSA_SLOW_ASSERT(this,
@@ -2336,7 +2337,7 @@ TNode<Uint32T> StringBuiltinsAssembler::LoadSurrogatePairAt(
 
     switch (encoding) {
       case UnicodeEncoding::UTF16:
-        var_result = Unsigned(Word32Or(
+        var_result = Signed(Word32Or(
 // Need to swap the order for big-endian platforms
 #if V8_TARGET_BIG_ENDIAN
             Word32Shl(lead, Int32Constant(16)), trail));
@@ -2352,8 +2353,8 @@ TNode<Uint32T> StringBuiltinsAssembler::LoadSurrogatePairAt(
             Int32Constant(0x10000 - (0xD800 << 10) - 0xDC00);
 
         // (lead << 10) + trail + SURROGATE_OFFSET
-        var_result = Unsigned(Int32Add(Word32Shl(lead, Int32Constant(10)),
-                                       Int32Add(trail, surrogate_offset)));
+        var_result = Signed(Int32Add(Word32Shl(lead, Int32Constant(10)),
+                                     Int32Add(trail, surrogate_offset)));
         break;
       }
     }
@@ -2392,8 +2393,8 @@ TF_BUILTIN(StringIteratorPrototypeNext, StringBuiltinsAssembler) {
   BIND(&next_codepoint);
   {
     UnicodeEncoding encoding = UnicodeEncoding::UTF16;
-    Node* ch = LoadSurrogatePairAt(string, length, position, encoding);
-    Node* value = StringFromCodePoint(ch, encoding);
+    TNode<Int32T> ch = LoadSurrogatePairAt(string, length, position, encoding);
+    TNode<String> value = StringFromCodePoint(ch, encoding);
     var_value.Bind(value);
     TNode<IntPtrT> length = LoadStringLengthAsWord(value);
     StoreObjectFieldNoWriteBarrier(iterator, JSStringIterator::kNextIndexOffset,

src/builtins/builtins-string-gen.h

@@ -57,10 +57,10 @@ class StringBuiltinsAssembler : public CodeStubAssembler {
                                  SloppyTNode<Object> value,
                                  SloppyTNode<Smi> limit);
 
-  TNode<Uint32T> LoadSurrogatePairAt(SloppyTNode<String> string,
-                                     SloppyTNode<IntPtrT> length,
-                                     SloppyTNode<IntPtrT> index,
-                                     UnicodeEncoding encoding);
+  TNode<Int32T> LoadSurrogatePairAt(SloppyTNode<String> string,
+                                    SloppyTNode<IntPtrT> length,
+                                    SloppyTNode<IntPtrT> index,
+                                    UnicodeEncoding encoding);
 
   void StringIndexOf(Node* const subject_string, Node* const search_string,
                      Node* const position, std::function<void(Node*)> f_return);

src/code-stub-assembler.cc

@@ -4498,14 +4498,14 @@ Node* CodeStubAssembler::IsNumberArrayIndex(Node* number) {
   return var_result.value();
 }
 
-TNode<Uint32T> CodeStubAssembler::StringCharCodeAt(SloppyTNode<String> string,
-                                                   SloppyTNode<IntPtrT> index) {
+TNode<Int32T> CodeStubAssembler::StringCharCodeAt(SloppyTNode<String> string,
+                                                  SloppyTNode<IntPtrT> index) {
   CSA_ASSERT(this, IsString(string));
 
   CSA_ASSERT(this, IntPtrGreaterThanOrEqual(index, IntPtrConstant(0)));
   CSA_ASSERT(this, IntPtrLessThan(index, LoadStringLengthAsWord(string)));
 
-  VARIABLE(var_result, MachineRepresentation::kWord32);
+  TVARIABLE(Int32T, var_result);
 
   Label return_result(this), if_runtime(this, Label::kDeferred),
       if_stringistwobyte(this), if_stringisonebyte(this);
@@ -4523,14 +4523,16 @@ TNode<Uint32T> CodeStubAssembler::StringCharCodeAt(SloppyTNode<String> string,
 
   BIND(&if_stringisonebyte);
   {
-    var_result.Bind(Load(MachineType::Uint8(), string_data, offset));
+    var_result =
+        UncheckedCast<Int32T>(Load(MachineType::Uint8(), string_data, offset));
     Goto(&return_result);
   }
 
   BIND(&if_stringistwobyte);
   {
-    var_result.Bind(Load(MachineType::Uint16(), string_data,
-                         WordShl(offset, IntPtrConstant(1))));
+    var_result =
+        UncheckedCast<Int32T>(Load(MachineType::Uint16(), string_data,
+                                   WordShl(offset, IntPtrConstant(1))));
     Goto(&return_result);
   }
 
@@ -4538,15 +4540,15 @@ TNode<Uint32T> CodeStubAssembler::StringCharCodeAt(SloppyTNode<String> string,
   {
     Node* result = CallRuntime(Runtime::kStringCharCodeAt, NoContextConstant(),
                                string, SmiTag(index));
-    var_result.Bind(SmiToWord32(result));
+    var_result = SmiToWord32(result);
     Goto(&return_result);
   }
 
   BIND(&return_result);
-  return UncheckedCast<Uint32T>(var_result.value());
+  return var_result;
 }
 
-Node* CodeStubAssembler::StringFromCharCode(Node* code) {
+TNode<String> CodeStubAssembler::StringFromCharCode(TNode<Int32T> code) {
   VARIABLE(var_result, MachineRepresentation::kTagged);
 
   // Check if the {code} is a one-byte char code.
@@ -4600,7 +4602,7 @@ Node* CodeStubAssembler::StringFromCharCode(Node* code) {
 
   BIND(&if_done);
   CSA_ASSERT(this, IsString(var_result.value()));
-  return var_result.value();
+  return CAST(var_result.value());
 }
 
 // A wrapper around CopyStringCharacters which determines the correct string
@@ -4760,7 +4762,7 @@ Node* CodeStubAssembler::SubString(Node* context, Node* string, Node* from,
   // Substrings of length 1 are generated through CharCodeAt and FromCharCode.
   BIND(&single_char);
   {
-    Node* char_code = StringCharCodeAt(string, SmiUntag(from));
+    TNode<Int32T> char_code = StringCharCodeAt(string, SmiUntag(from));
     var_result.Bind(StringFromCharCode(char_code));
     Goto(&end);
   }
@@ -5134,8 +5136,8 @@ Node* CodeStubAssembler::StringAdd(Node* context, Node* left, Node* right,
   return result.value();
 }
 
-Node* CodeStubAssembler::StringFromCodePoint(Node* codepoint,
-                                             UnicodeEncoding encoding) {
+TNode<String> CodeStubAssembler::StringFromCodePoint(TNode<Int32T> codepoint,
+                                                     UnicodeEncoding encoding) {
   VARIABLE(var_result, MachineRepresentation::kTagged, EmptyStringConstant());
 
   Label if_isword16(this), if_isword32(this), return_result(this);
@@ -5167,7 +5169,7 @@ Node* CodeStubAssembler::StringFromCodePoint(Node* codepoint,
                                Int32Constant(0xDC00));
 
         // codpoint = (trail << 16) | lead;
-        codepoint = Word32Or(Word32Shl(trail, Int32Constant(16)), lead);
+        codepoint = Signed(Word32Or(Word32Shl(trail, Int32Constant(16)), lead));
         break;
       }
     }
@@ -5182,8 +5184,7 @@ Node* CodeStubAssembler::StringFromCodePoint(Node* codepoint,
   }
 
   BIND(&return_result);
-  CSA_ASSERT(this, IsString(var_result.value()));
-  return var_result.value();
+  return CAST(var_result.value());
 }
 
 TNode<Number> CodeStubAssembler::StringToNumber(SloppyTNode<Context> context,
@@ -7064,7 +7065,7 @@ void CodeStubAssembler::BranchIfMaybeSpecialIndex(TNode<String> name_string,
 
   // If the first character of name is not a digit or '-', or we can't match it
   // to Infinity or NaN, then this is not a special index.
-  TNode<Uint32T> first_char = StringCharCodeAt(name_string, IntPtrConstant(0));
+  TNode<Int32T> first_char = StringCharCodeAt(name_string, IntPtrConstant(0));
   // If the name starts with '-', it can be a negative index.
   GotoIf(Word32Equal(first_char, Int32Constant('-')), if_maybe_special_index);
   // If the name starts with 'I', it can be "Infinity".

src/code-stub-assembler.h

@@ -1164,10 +1164,10 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
 
   // String helpers.
   // Load a character from a String (might flatten a ConsString).
-  TNode<Uint32T> StringCharCodeAt(SloppyTNode<String> string,
-                                  SloppyTNode<IntPtrT> index);
+  TNode<Int32T> StringCharCodeAt(SloppyTNode<String> string,
+                                 SloppyTNode<IntPtrT> index);
   // Return the single character string with only {code}.
-  Node* StringFromCharCode(Node* code);
+  TNode<String> StringFromCharCode(TNode<Int32T> code);
 
   enum class SubStringFlags { NONE, FROM_TO_ARE_BOUNDED };
 
@@ -1198,7 +1198,8 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                                  Variable* var_right, Node* right_instance_type,
                                  Label* did_something);
 
-  Node* StringFromCodePoint(Node* codepoint, UnicodeEncoding encoding);
+  TNode<String> StringFromCodePoint(TNode<Int32T> codepoint,
+                                    UnicodeEncoding encoding);
 
   // Type conversion helpers.
   enum class BigIntHandling { kConvertToNumber, kThrow };

src/ic/accessor-assembler.cc

@@ -338,8 +338,8 @@ void AccessorAssembler::HandleLoadICSmiHandlerCase(
       Node* intptr_index = TryToIntptr(p->name, miss);
       Node* length = LoadStringLengthAsWord(holder);
       GotoIf(UintPtrGreaterThanOrEqual(intptr_index, length), &if_oob);
-      Node* code = StringCharCodeAt(holder, intptr_index);
-      Node* result = StringFromCharCode(code);
+      TNode<Int32T> code = StringCharCodeAt(holder, intptr_index);
+      TNode<String> result = StringFromCharCode(code);
       Return(result);
 
       BIND(&if_oob);