[turbofan] Only poison loads and branches participating in property access

This cuts down the perf cost on Octane from 18% to 13%. The baseline is the no mitigation
Octane score, the array access mitigation cost was about 4%. This means we would be
getting a bit more than 1/3 of the poisoning regression back.

Bug: chromium:856973, chromium:887213
Change-Id: Ibd99f66ae832c6080f2c2e5b33a1a7610907466f
Reviewed-on: https://chromium-review.googlesource.com/c/1251401Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56409}

src/compiler/access-builder.cc

@@ -67,20 +67,20 @@ FieldAccess AccessBuilder::ForBigIntBitfield() {
 
 // static
 FieldAccess AccessBuilder::ForJSObjectPropertiesOrHash() {
-  FieldAccess access = {kTaggedBase,         JSObject::kPropertiesOrHashOffset,
-                        MaybeHandle<Name>(), MaybeHandle<Map>(),
-                        Type::Any(),         MachineType::AnyTagged(),
-                        kPointerWriteBarrier};
+  FieldAccess access = {kTaggedBase,          JSObject::kPropertiesOrHashOffset,
+                        MaybeHandle<Name>(),  MaybeHandle<Map>(),
+                        Type::Any(),          MachineType::AnyTagged(),
+                        kPointerWriteBarrier, LoadSensitivity::kCritical};
   return access;
 }
 
 
 // static
 FieldAccess AccessBuilder::ForJSObjectElements() {
-  FieldAccess access = {kTaggedBase,         JSObject::kElementsOffset,
-                        MaybeHandle<Name>(), MaybeHandle<Map>(),
-                        Type::Internal(),    MachineType::TaggedPointer(),
-                        kPointerWriteBarrier};
+  FieldAccess access = {kTaggedBase,          JSObject::kElementsOffset,
+                        MaybeHandle<Name>(),  MaybeHandle<Map>(),
+                        Type::Internal(),     MachineType::TaggedPointer(),
+                        kPointerWriteBarrier, LoadSensitivity::kCritical};
   return access;
 }
 
@@ -494,7 +494,7 @@ FieldAccess AccessBuilder::ForFixedTypedArrayBaseBasePointer() {
       kTaggedBase,           FixedTypedArrayBase::kBasePointerOffset,
       MaybeHandle<Name>(),   MaybeHandle<Map>(),
       Type::OtherInternal(), MachineType::AnyTagged(),
-      kPointerWriteBarrier};
+      kPointerWriteBarrier,  LoadSensitivity::kCritical};
   return access;
 }
 
@@ -506,7 +506,8 @@ FieldAccess AccessBuilder::ForFixedTypedArrayBaseExternalPointer() {
                         MaybeHandle<Map>(),
                         Type::ExternalPointer(),
                         MachineType::Pointer(),
-                        kNoWriteBarrier};
+                        kNoWriteBarrier,
+                        LoadSensitivity::kCritical};
   return access;
 }
 
@@ -835,10 +836,10 @@ FieldAccess AccessBuilder::ForFixedArraySlot(
 
 // static
 FieldAccess AccessBuilder::ForCellValue() {
-  FieldAccess access = {kTaggedBase,      Cell::kValueOffset,
-                        Handle<Name>(),   MaybeHandle<Map>(),
-                        Type::Any(),      MachineType::AnyTagged(),
-                        kFullWriteBarrier};
+  FieldAccess access = {kTaggedBase,       Cell::kValueOffset,
+                        Handle<Name>(),    MaybeHandle<Map>(),
+                        Type::Any(),       MachineType::AnyTagged(),
+                        kFullWriteBarrier, LoadSensitivity::kCritical};
   return access;
 }
 

src/compiler/effect-control-linearizer.cc

@@ -1462,10 +1462,11 @@ void EffectControlLinearizer::LowerCheckMaps(Node* node, Node* frame_state) {
       Node* map = __ HeapConstant(maps[i]);
       Node* check = __ WordEqual(value_map, map);
       if (i == map_count - 1) {
-        __ GotoIfNot(check, &migrate);
-        __ Goto(&done);
+        __ Branch(check, &done, &migrate, IsSafetyCheck::kCriticalSafetyCheck);
       } else {
-        __ GotoIf(check, &done);
+        auto next_map = __ MakeLabel();
+        __ Branch(check, &done, &next_map, IsSafetyCheck::kCriticalSafetyCheck);
+        __ Bind(&next_map);
       }
     }
 
@@ -1480,7 +1481,8 @@ void EffectControlLinearizer::LowerCheckMaps(Node* node, Node* frame_state) {
                        __ Int32Constant(Map::IsDeprecatedBit::kMask)),
           __ Int32Constant(0));
       __ DeoptimizeIf(DeoptimizeReason::kWrongMap, p.feedback(),
-                      if_not_deprecated, frame_state);
+                      if_not_deprecated, frame_state,
+                      IsSafetyCheck::kCriticalSafetyCheck);
 
       Operator::Properties properties = Operator::kNoDeopt | Operator::kNoThrow;
       Runtime::FunctionId id = Runtime::kTryMigrateInstance;
@@ -1491,7 +1493,7 @@ void EffectControlLinearizer::LowerCheckMaps(Node* node, Node* frame_state) {
                              __ Int32Constant(1), __ NoContextConstant());
       Node* check = ObjectIsSmi(result);
       __ DeoptimizeIf(DeoptimizeReason::kInstanceMigrationFailed, p.feedback(),
-                      check, frame_state);
+                      check, frame_state, IsSafetyCheck::kCriticalSafetyCheck);
     }
 
     // Reload the current map of the {value}.
@@ -1503,9 +1505,11 @@ void EffectControlLinearizer::LowerCheckMaps(Node* node, Node* frame_state) {
       Node* check = __ WordEqual(value_map, map);
       if (i == map_count - 1) {
         __ DeoptimizeIfNot(DeoptimizeReason::kWrongMap, p.feedback(), check,
-                           frame_state);
+                           frame_state, IsSafetyCheck::kCriticalSafetyCheck);
       } else {
-        __ GotoIf(check, &done);
+        auto next_map = __ MakeLabel();
+        __ Branch(check, &done, &next_map, IsSafetyCheck::kCriticalSafetyCheck);
+        __ Bind(&next_map);
       }
     }
 
@@ -1522,9 +1526,11 @@ void EffectControlLinearizer::LowerCheckMaps(Node* node, Node* frame_state) {
       Node* check = __ WordEqual(value_map, map);
       if (i == map_count - 1) {
         __ DeoptimizeIfNot(DeoptimizeReason::kWrongMap, p.feedback(), check,
-                           frame_state);
+                           frame_state, IsSafetyCheck::kCriticalSafetyCheck);
       } else {
-        __ GotoIf(check, &done);
+        auto next_map = __ MakeLabel();
+        __ Branch(check, &done, &next_map, IsSafetyCheck::kCriticalSafetyCheck);
+        __ Bind(&next_map);
       }
     }
     __ Goto(&done);
@@ -1545,7 +1551,14 @@ Node* EffectControlLinearizer::LowerCompareMaps(Node* node) {
   for (size_t i = 0; i < map_count; ++i) {
     Node* map = __ HeapConstant(maps[i]);
     Node* check = __ WordEqual(value_map, map);
-    __ GotoIf(check, &done, __ Int32Constant(1));
+    auto next_map = __ MakeLabel();
+    auto passed = __ MakeLabel();
+    __ Branch(check, &passed, &next_map, IsSafetyCheck::kCriticalSafetyCheck);
+
+    __ Bind(&passed);
+    __ Goto(&done, __ Int32Constant(1));
+
+    __ Bind(&next_map);
   }
   __ Goto(&done, __ Int32Constant(0));
 

src/compiler/graph-assembler.cc

@@ -214,9 +214,11 @@ Node* GraphAssembler::Word32PoisonOnSpeculation(Node* value) {
 
 Node* GraphAssembler::DeoptimizeIf(DeoptimizeReason reason,
                                    VectorSlotPair const& feedback,
-                                   Node* condition, Node* frame_state) {
+                                   Node* condition, Node* frame_state,
+                                   IsSafetyCheck is_safety_check) {
   return current_control_ = current_effect_ = graph()->NewNode(
-             common()->DeoptimizeIf(DeoptimizeKind::kEager, reason, feedback),
+             common()->DeoptimizeIf(DeoptimizeKind::kEager, reason, feedback,
+                                    is_safety_check),
              condition, frame_state, current_effect_, current_control_);
 }
 
@@ -231,7 +233,8 @@ Node* GraphAssembler::DeoptimizeIfNot(DeoptimizeReason reason,
 }
 
 void GraphAssembler::Branch(Node* condition, GraphAssemblerLabel<0u>* if_true,
-                            GraphAssemblerLabel<0u>* if_false) {
+                            GraphAssemblerLabel<0u>* if_false,
+                            IsSafetyCheck is_safety_check) {
   DCHECK_NOT_NULL(current_control_);
 
   BranchHint hint = BranchHint::kNone;
@@ -239,8 +242,8 @@ void GraphAssembler::Branch(Node* condition, GraphAssemblerLabel<0u>* if_true,
     hint = if_false->IsDeferred() ? BranchHint::kTrue : BranchHint::kFalse;
   }
 
-  Node* branch =
-      graph()->NewNode(common()->Branch(hint), condition, current_control_);
+  Node* branch = graph()->NewNode(common()->Branch(hint, is_safety_check),
+                                  condition, current_control_);
 
   current_control_ = graph()->NewNode(common()->IfTrue(), branch);
   MergeState(if_true);

src/compiler/graph-assembler.h

@@ -234,8 +234,10 @@ class GraphAssembler {
 
   Node* Word32PoisonOnSpeculation(Node* value);
 
-  Node* DeoptimizeIf(DeoptimizeReason reason, VectorSlotPair const& feedback,
-                     Node* condition, Node* frame_state);
+  Node* DeoptimizeIf(
+      DeoptimizeReason reason, VectorSlotPair const& feedback, Node* condition,
+      Node* frame_state,
+      IsSafetyCheck is_safety_check = IsSafetyCheck::kSafetyCheck);
   Node* DeoptimizeIfNot(
       DeoptimizeReason reason, VectorSlotPair const& feedback, Node* condition,
       Node* frame_state,
@@ -253,7 +255,8 @@ class GraphAssembler {
   void Goto(GraphAssemblerLabel<sizeof...(Vars)>* label, Vars...);
 
   void Branch(Node* condition, GraphAssemblerLabel<0u>* if_true,
-              GraphAssemblerLabel<0u>* if_false);
+              GraphAssemblerLabel<0u>* if_false,
+              IsSafetyCheck is_safety_check = IsSafetyCheck::kNoSafetyCheck);
 
   // Control helpers.
   // {GotoIf(c, l)} is equivalent to {Branch(c, l, templ);Bind(templ)}.

src/compiler/pipeline.cc

@@ -916,9 +916,9 @@ PipelineCompilationJob::Status PipelineCompilationJob::PrepareJobImpl(
   PoisoningMitigationLevel load_poisoning =
       PoisoningMitigationLevel::kDontPoison;
   if (FLAG_untrusted_code_mitigations) {
-    // For partial mitigations, this can be changed to
-    // PoisoningMitigationLevel::kPoisonCriticalOnly.
-    load_poisoning = PoisoningMitigationLevel::kPoisonAll;
+    // For full mitigations, this can be changed to
+    // PoisoningMitigationLevel::kPoisonAll.
+    load_poisoning = PoisoningMitigationLevel::kPoisonCriticalOnly;
   }
   compilation_info()->SetPoisoningMitigationLevel(load_poisoning);
 

src/compiler/property-access-builder.cc

@@ -246,7 +246,8 @@ Node* PropertyAccessBuilder::BuildLoadDataField(
       MaybeHandle<Map>(),
       field_type,
       MachineType::TypeForRepresentation(field_representation),
-      kFullWriteBarrier};
+      kFullWriteBarrier,
+      LoadSensitivity::kCritical};
   if (field_representation == MachineRepresentation::kFloat64) {
     if (!field_index.is_inobject() || field_index.is_hidden_field() ||
         !FLAG_unbox_double_fields) {
@@ -256,7 +257,8 @@ Node* PropertyAccessBuilder::BuildLoadDataField(
                                           MaybeHandle<Map>(),
                                           Type::OtherInternal(),
                                           MachineType::TaggedPointer(),
-                                          kPointerWriteBarrier};
+                                          kPointerWriteBarrier,
+                                          LoadSensitivity::kCritical};
       storage = *effect = graph()->NewNode(
           simplified()->LoadField(storage_access), storage, *effect, *control);
       field_access.offset = HeapNumber::kValueOffset;