[compiler] Introduce a new node for array index masking.

Bug: 
Change-Id: Idf3829f59cebe5c68b011ab6a0fa766ca2bad359
Reviewed-on: https://chromium-review.googlesource.com/832448
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50231}

src/compiler/effect-control-linearizer.cc

@@ -21,10 +21,12 @@ namespace compiler {
 
 EffectControlLinearizer::EffectControlLinearizer(
     JSGraph* js_graph, Schedule* schedule, Zone* temp_zone,
-    SourcePositionTable* source_positions)
+    SourcePositionTable* source_positions,
+    MaskArrayIndexEnable mask_array_index)
     : js_graph_(js_graph),
       schedule_(schedule),
       temp_zone_(temp_zone),
+      mask_array_index_(mask_array_index),
       source_positions_(source_positions),
       graph_assembler_(js_graph, nullptr, nullptr, temp_zone),
       frame_state_zapper_(nullptr) {}
@@ -690,6 +692,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
     case IrOpcode::kCheckBounds:
       result = LowerCheckBounds(node, frame_state);
       break;
+    case IrOpcode::kMaskIndexWithBound:
+      result = LowerMaskIndexWithBound(node);
+      break;
     case IrOpcode::kCheckMaps:
       LowerCheckMaps(node, frame_state);
       break;
@@ -1320,6 +1325,19 @@ Node* EffectControlLinearizer::LowerCheckBounds(Node* node, Node* frame_state) {
   return index;
 }
 
+Node* EffectControlLinearizer::LowerMaskIndexWithBound(Node* node) {
+  Node* index = node->InputAt(0);
+  if (mask_array_index_ == kMaskArrayIndex) {
+    Node* limit = node->InputAt(1);
+
+    Node* mask = __ Word32Sar(__ Word32Or(__ Int32Sub(limit, index), index),
+                              __ Int32Constant(31));
+    mask = __ Word32Xor(mask, __ Int32Constant(-1));
+    index = __ Word32And(index, mask);
+  }
+  return index;
+}
+
 void EffectControlLinearizer::LowerCheckMaps(Node* node, Node* frame_state) {
   CheckMapsParameters const& p = CheckMapsParametersOf(node->op());
   Node* value = node->InputAt(0);

src/compiler/effect-control-linearizer.h

@@ -30,8 +30,11 @@ class SourcePositionTable;
 
 class V8_EXPORT_PRIVATE EffectControlLinearizer {
  public:
+  enum MaskArrayIndexEnable { kDoNotMaskArrayIndex, kMaskArrayIndex };
+
   EffectControlLinearizer(JSGraph* graph, Schedule* schedule, Zone* temp_zone,
-                          SourcePositionTable* source_positions);
+                          SourcePositionTable* source_positions,
+                          MaskArrayIndexEnable mask_array_index);
 
   void Run();
 
@@ -53,6 +56,7 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer {
   Node* LowerChangeTaggedToUint32(Node* node);
   Node* LowerChangeTaggedToTaggedSigned(Node* node);
   Node* LowerCheckBounds(Node* node, Node* frame_state);
+  Node* LowerMaskIndexWithBound(Node* node);
   Node* LowerCheckInternalizedString(Node* node, Node* frame_state);
   void LowerCheckMaps(Node* node, Node* frame_state);
   Node* LowerCompareMaps(Node* node);
@@ -194,6 +198,7 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer {
   JSGraph* js_graph_;
   Schedule* schedule_;
   Zone* temp_zone_;
+  MaskArrayIndexEnable mask_array_index_;
   RegionObservability region_observability_ = RegionObservability::kObservable;
   SourcePositionTable* source_positions_;
   GraphAssembler graph_assembler_;

src/compiler/graph-assembler.h

@@ -41,6 +41,7 @@ namespace compiler {
   V(Word32Xor)                            \
   V(Word32Shr)                            \
   V(Word32Shl)                            \
+  V(Word32Sar)                            \
   V(IntAdd)                               \
   V(IntSub)                               \
   V(IntMul)                               \

src/compiler/js-builtin-reducer.cc

@@ -2405,8 +2405,12 @@ Reduction JSBuiltinReducer::ReduceStringCharAt(Node* node) {
 
         // Return the character from the {receiver} as single character string.
         Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+
+        Node* masked_index = graph()->NewNode(
+            simplified()->MaskIndexWithBound(), index, receiver_length);
+
         Node* vtrue = graph()->NewNode(simplified()->StringCharAt(), receiver,
-                                       index, if_true);
+                                       masked_index, if_true);
 
         // Return the empty string otherwise.
         Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
@@ -2458,8 +2462,12 @@ Reduction JSBuiltinReducer::ReduceStringCharCodeAt(Node* node) {
 
         // Load the character from the {receiver}.
         Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+
+        Node* masked_index = graph()->NewNode(
+            simplified()->MaskIndexWithBound(), index, receiver_length);
+
         Node* vtrue = graph()->NewNode(simplified()->StringCharCodeAt(),
-                                       receiver, index, if_true);
+                                       receiver, masked_index, if_true);
 
         // Return NaN otherwise.
         Node* if_false = graph()->NewNode(common()->IfFalse(), branch);

src/compiler/js-call-reducer.cc

@@ -1722,9 +1722,12 @@ Node* JSCallReducer::SafeLoadElement(ElementsKind kind, Node* receiver,
       simplified()->LoadField(AccessBuilder::ForJSObjectElements()), receiver,
       *effect, control);
 
+  Node* masked_index =
+      graph()->NewNode(simplified()->MaskIndexWithBound(), *k, length);
+
   Node* element = *effect = graph()->NewNode(
       simplified()->LoadElement(AccessBuilder::ForFixedArrayElement(kind)),
-      elements, *k, *effect, control);
+      elements, masked_index, *effect, control);
   return element;
 }
 

src/compiler/js-native-context-specialization.cc

@@ -2153,10 +2153,13 @@ JSNativeContextSpecialization::BuildElementAccess(
           Node* etrue = effect;
           Node* vtrue;
           {
+            Node* masked_index = graph()->NewNode(
+                simplified()->MaskIndexWithBound(), index, length);
+
             // Perform the actual load
             vtrue = etrue = graph()->NewNode(
                 simplified()->LoadTypedElement(external_array_type), buffer,
-                base_pointer, external_pointer, index, etrue, if_true);
+                base_pointer, external_pointer, masked_index, etrue, if_true);
           }
 
           Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
@@ -2174,10 +2177,13 @@ JSNativeContextSpecialization::BuildElementAccess(
               graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
                                vtrue, vfalse, control);
         } else {
+          Node* masked_index = graph()->NewNode(
+              simplified()->MaskIndexWithBound(), index, length);
+
           // Perform the actual load.
           value = effect = graph()->NewNode(
               simplified()->LoadTypedElement(external_array_type), buffer,
-              base_pointer, external_pointer, index, effect, control);
+              base_pointer, external_pointer, masked_index, effect, control);
         }
         break;
       }
@@ -2323,10 +2329,13 @@ JSNativeContextSpecialization::BuildElementAccess(
         Node* etrue = effect;
         Node* vtrue;
         {
+          Node* masked_index = graph()->NewNode(
+              simplified()->MaskIndexWithBound(), index, length);
+
           // Perform the actual load
           vtrue = etrue =
               graph()->NewNode(simplified()->LoadElement(element_access),
-                               elements, index, etrue, if_true);
+                               elements, masked_index, etrue, if_true);
 
           // Handle loading from holey backing stores correctly, by either
           // mapping the hole to undefined if possible, or deoptimizing
@@ -2361,10 +2370,13 @@ JSNativeContextSpecialization::BuildElementAccess(
             graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
                              vtrue, vfalse, control);
       } else {
+        Node* masked_index =
+            graph()->NewNode(simplified()->MaskIndexWithBound(), index, length);
+
         // Perform the actual load.
         value = effect =
             graph()->NewNode(simplified()->LoadElement(element_access),
-                             elements, index, effect, control);
+                             elements, masked_index, effect, control);
 
         // Handle loading from holey backing stores correctly, by either mapping
         // the hole to undefined if possible, or deoptimizing otherwise.
@@ -2508,9 +2520,12 @@ Node* JSNativeContextSpecialization::BuildIndexedStringLoad(
     Node* branch =
         graph()->NewNode(common()->Branch(BranchHint::kTrue), check, *control);
 
+    Node* masked_index =
+        graph()->NewNode(simplified()->MaskIndexWithBound(), index, length);
+
     Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
     Node* vtrue = graph()->NewNode(simplified()->StringCharAt(), receiver,
-                                   index, if_true);
+                                   masked_index, if_true);
 
     Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
     Node* vfalse = jsgraph()->UndefinedConstant();
@@ -2524,9 +2539,12 @@ Node* JSNativeContextSpecialization::BuildIndexedStringLoad(
         graph()->NewNode(simplified()->CheckBounds(VectorSlotPair()), index,
                          length, *effect, *control);
 
+    Node* masked_index =
+        graph()->NewNode(simplified()->MaskIndexWithBound(), index, length);
+
     // Return the character from the {receiver} as single character string.
-    return graph()->NewNode(simplified()->StringCharAt(), receiver, index,
-                            *control);
+    return graph()->NewNode(simplified()->StringCharAt(), receiver,
+                            masked_index, *control);
   }
 }
 

src/compiler/opcodes.h

@@ -398,6 +398,7 @@
   V(TransitionElementsKind)             \
   V(FindOrderedHashMapEntry)            \
   V(FindOrderedHashMapEntryForInt32Key) \
+  V(MaskIndexWithBound)                 \
   V(RuntimeAbort)
 
 #define SIMPLIFIED_OP_LIST(V)                 \

src/compiler/pipeline.cc

@@ -1371,8 +1371,12 @@ struct EffectControlLinearizationPhase {
       //   chains and lower them,
       // - get rid of the region markers,
       // - introduce effect phis and rewire effects to get SSA again.
+      EffectControlLinearizer::MaskArrayIndexEnable mask_array_index =
+          FLAG_mask_array_index ? EffectControlLinearizer::kMaskArrayIndex
+                                : EffectControlLinearizer::kDoNotMaskArrayIndex;
       EffectControlLinearizer linearizer(data->jsgraph(), schedule, temp_zone,
-                                         data->source_positions());
+                                         data->source_positions(),
+                                         mask_array_index);
       linearizer.Run();
     }
     {

src/compiler/simplified-lowering.cc

@@ -2397,6 +2397,11 @@ class RepresentationSelector {
         }
         return;
       }
+      case IrOpcode::kMaskIndexWithBound: {
+        VisitBinop(node, UseInfo::TruncatingWord32(),
+                   MachineRepresentation::kWord32);
+        return;
+      }
       case IrOpcode::kCheckHeapObject: {
         if (InputCannotBe(node, Type::SignedSmall())) {
           VisitUnop(node, UseInfo::AnyTagged(),

src/compiler/simplified-operator.cc

@@ -703,7 +703,8 @@ bool operator==(CheckMinusZeroParameters const& lhs,
   V(StringLessThan, Operator::kNoProperties, 2, 0)               \
   V(StringLessThanOrEqual, Operator::kNoProperties, 2, 0)        \
   V(ToBoolean, Operator::kNoProperties, 1, 0)                    \
-  V(NewConsString, Operator::kNoProperties, 3, 0)
+  V(NewConsString, Operator::kNoProperties, 3, 0)                \
+  V(MaskIndexWithBound, Operator::kNoProperties, 2, 0)
 
 #define SPECULATIVE_NUMBER_BINOP_LIST(V)      \
   SIMPLIFIED_SPECULATIVE_NUMBER_BINOP_LIST(V) \

src/compiler/simplified-operator.h

@@ -539,6 +539,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
   const Operator* CheckBounds(const VectorSlotPair& feedback);
   const Operator* CheckMaps(CheckMapsFlags, ZoneHandleSet<Map>,
                             const VectorSlotPair& = VectorSlotPair());
+  const Operator* MaskIndexWithBound();
   const Operator* CompareMaps(ZoneHandleSet<Map>);
   const Operator* MapGuard(ZoneHandleSet<Map> maps);
 

src/compiler/typer.cc

@@ -1958,6 +1958,10 @@ Type* Typer::Visitor::TypeStringLength(Node* node) {
   return typer_->cache_.kStringLengthType;
 }
 
+Type* Typer::Visitor::TypeMaskIndexWithBound(Node* node) {
+  return Type::Union(Operand(node, 0), typer_->cache_.kSingletonZero, zone());
+}
+
 Type* Typer::Visitor::TypeCheckBounds(Node* node) {
   Type* index = Operand(node, 0);
   Type* length = Operand(node, 1);

src/compiler/verifier.cc

@@ -1277,6 +1277,11 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) {
       CheckValueInputIs(node, 1, Type::Unsigned31());
       CheckTypeIs(node, Type::Unsigned31());
       break;
+    case IrOpcode::kMaskIndexWithBound:
+      CheckValueInputIs(node, 0, Type::Unsigned32());
+      CheckValueInputIs(node, 1, Type::Unsigned31());
+      CheckTypeIs(node, Type::Unsigned32());
+      break;
     case IrOpcode::kCheckHeapObject:
       CheckValueInputIs(node, 0, Type::Any());
       break;

src/flag-definitions.h

@@ -463,6 +463,9 @@ DEFINE_BOOL(turbo_store_elimination, true,
 DEFINE_BOOL(trace_store_elimination, false, "trace store elimination")
 DEFINE_BOOL(turbo_rewrite_far_jumps, true,
             "rewrite far to near jumps (ia32,x64)")
+DEFINE_BOOL(extra_masking, false, "Extra mask for memory accesses")
+DEFINE_BOOL(mask_array_index, false, "Mask array index with bound")
+DEFINE_IMPLICATION(extra_masking, mask_array_index)
 
 // Flags to help platform porters
 DEFINE_BOOL(minimal, false,

test/unittests/compiler/effect-control-linearizer-unittest.cc

@@ -81,8 +81,9 @@ TEST_F(EffectControlLinearizerTest, SimpleLoad) {
   schedule.AddReturn(start, ret);
 
   // Run the state effect introducer.
-  EffectControlLinearizer introducer(jsgraph(), &schedule, zone(),
-                                     source_positions());
+  EffectControlLinearizer introducer(
+      jsgraph(), &schedule, zone(), source_positions(),
+      EffectControlLinearizer::kDoNotMaskArrayIndex);
   introducer.Run();
 
   EXPECT_THAT(load,
@@ -143,8 +144,9 @@ TEST_F(EffectControlLinearizerTest, DiamondLoad) {
   schedule.AddReturn(mblock, ret);
 
   // Run the state effect introducer.
-  EffectControlLinearizer introducer(jsgraph(), &schedule, zone(),
-                                     source_positions());
+  EffectControlLinearizer introducer(
+      jsgraph(), &schedule, zone(), source_positions(),
+      EffectControlLinearizer::kDoNotMaskArrayIndex);
   introducer.Run();
 
   // The effect input to the return should be an effect phi with the
@@ -210,8 +212,9 @@ TEST_F(EffectControlLinearizerTest, LoopLoad) {
   schedule.AddReturn(rblock, ret);
 
   // Run the state effect introducer.
-  EffectControlLinearizer introducer(jsgraph(), &schedule, zone(),
-                                     source_positions());
+  EffectControlLinearizer introducer(
+      jsgraph(), &schedule, zone(), source_positions(),
+      EffectControlLinearizer::kDoNotMaskArrayIndex);
   introducer.Run();
 
   ASSERT_THAT(ret, IsReturn(load, load, if_true));
@@ -273,8 +276,9 @@ TEST_F(EffectControlLinearizerTest, CloneBranch) {
   schedule.AddNode(mblock, merge);
   schedule.AddNode(mblock, graph()->end());
 
-  EffectControlLinearizer introducer(jsgraph(), &schedule, zone(),
-                                     source_positions());
+  EffectControlLinearizer introducer(
+      jsgraph(), &schedule, zone(), source_positions(),
+      EffectControlLinearizer::kDoNotMaskArrayIndex);
   introducer.Run();
 
   Capture<Node *> branch1_capture, branch2_capture;