Reland "[turboprop] Pass required parameters as value inputs to TierUpCheck node"

This is a reland of 44f46def with a
fix for failures with --turbonci_as_mid_tier

Original change's description:
> [turboprop] Pass required parameters as value inputs to TierUpCheck node
>
> TierUpCheck node tail calls interpreter entry trampoline when additional
> processing is needed for tiering up. Calling IET requires target,
> new_target, input count and context as parameters. Earlier these were
> created as parameter nodes in effect-control-linearizer. This causes
> problems with Turboprop since TurboProp doesn't use the second scheduler
> and cannot reschedule these nodes to the start block. We should instead
> create these parameter nodes in bytecode-graph-builder and pass them
> as value inputs to TierUpCheck node.
>
> Bug: v8:9684
> Change-Id: Icfe5a33b4e628d5a3ba9a3121b2b0746be6aed5c
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2498695
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Commit-Queue: Mythri Alle <mythria@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70790}

Bug: v8:9684
Change-Id: Ic1a7d39aab0a599d0dd421f237e7bc640fcd6eb1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2504258
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70856}

src/compiler/backend/instruction-selector.cc

@@ -1332,6 +1332,8 @@ void InstructionSelector::VisitNode(Node* node) {
     case IrOpcode::kFinishRegion:
       return MarkAsTagged(node), VisitFinishRegion(node);
     case IrOpcode::kParameter: {
+      // Parameters should always be scheduled to the first block.
+      DCHECK_EQ(schedule()->block(node)->rpo_number(), 0);
       MachineType type =
           linkage()->GetParameterType(ParameterIndexOf(node->op()));
       MarkAsRepresentation(type.representation(), node);

src/compiler/bytecode-graph-builder.cc

@@ -1117,7 +1117,23 @@ Node* BytecodeGraphBuilder::BuildLoadNativeContext() {
 
 void BytecodeGraphBuilder::MaybeBuildTierUpCheck() {
   if (!CodeKindChecksOptimizationMarker(code_kind())) return;
-  NewNode(simplified()->TierUpCheck(), feedback_vector_node());
+
+  int parameter_count = bytecode_array().parameter_count();
+  Node* target = GetFunctionClosure();
+  Node* new_target = graph()->NewNode(
+      common()->Parameter(
+          Linkage::GetJSCallNewTargetParamIndex(parameter_count),
+          "%new.target"),
+      graph()->start());
+  Node* argc = graph()->NewNode(
+      common()->Parameter(Linkage::GetJSCallArgCountParamIndex(parameter_count),
+                          "%argc"),
+      graph()->start());
+  DCHECK_EQ(environment()->Context()->opcode(), IrOpcode::kParameter);
+  Node* context = environment()->Context();
+
+  NewNode(simplified()->TierUpCheck(), feedback_vector_node(), target,
+          new_target, argc, context);
 }
 
 void BytecodeGraphBuilder::MaybeBuildIncrementInvocationCount() {

src/compiler/effect-control-linearizer.cc

@@ -3713,17 +3713,8 @@ void EffectControlLinearizer::LowerTierUpCheck(Node* node) {
   // Currently we delegate these tasks to the InterpreterEntryTrampoline.
   // TODO(jgruber,v8:8888): Consider a dedicated builtin instead.
 
-  const int parameter_count =
-      StartNode{graph()->start()}.FormalParameterCount();
   TNode<HeapObject> code =
       __ HeapConstant(BUILTIN_CODE(isolate(), InterpreterEntryTrampoline));
-  Node* target = __ Parameter(Linkage::kJSCallClosureParamIndex);
-  Node* new_target =
-      __ Parameter(Linkage::GetJSCallNewTargetParamIndex(parameter_count));
-  Node* argc =
-      __ Parameter(Linkage::GetJSCallArgCountParamIndex(parameter_count));
-  Node* context =
-      __ Parameter(Linkage::GetJSCallContextParamIndex(parameter_count));
 
   JSTrampolineDescriptor descriptor;
   CallDescriptor::Flags flags = CallDescriptor::kFixedTargetRegister |
@@ -3731,8 +3722,8 @@ void EffectControlLinearizer::LowerTierUpCheck(Node* node) {
   auto call_descriptor = Linkage::GetStubCallDescriptor(
       graph()->zone(), descriptor, descriptor.GetStackParameterCount(), flags,
       Operator::kNoProperties);
-  Node* nodes[] = {code,    target,      new_target,  argc,
-                   context, __ effect(), __ control()};
+  Node* nodes[] = {code,        n.target(),  n.new_target(), n.input_count(),
+                   n.context(), __ effect(), __ control()};
 
 #ifdef DEBUG
   static constexpr int kCodeContextEffectControl = 4;

src/compiler/pipeline.cc

@@ -1668,11 +1668,11 @@ struct TypeAssertionsPhase {
 struct SimplifiedLoweringPhase {
   DECL_PIPELINE_PHASE_CONSTANTS(SimplifiedLowering)
 
-  void Run(PipelineData* data, Zone* temp_zone) {
+  void Run(PipelineData* data, Zone* temp_zone, Linkage* linkage) {
     SimplifiedLowering lowering(data->jsgraph(), data->broker(), temp_zone,
                                 data->source_positions(), data->node_origins(),
                                 data->info()->GetPoisoningMitigationLevel(),
-                                &data->info()->tick_counter());
+                                &data->info()->tick_counter(), linkage);
 
     // RepresentationChanger accesses the heap.
     UnparkedScopeIfNeeded scope(data->broker());
@@ -2561,7 +2561,7 @@ bool PipelineImpl::OptimizeGraph(Linkage* linkage) {
   // Perform simplified lowering. This has to run w/o the Typer decorator,
   // because we cannot compute meaningful types anyways, and the computed types
   // might even conflict with the representation/truncation logic.
-  Run<SimplifiedLoweringPhase>();
+  Run<SimplifiedLoweringPhase>(linkage);
   RunPrintAndVerify(SimplifiedLoweringPhase::phase_name(), true);
 
   // From now on it is invalid to look at types on the nodes, because the types
@@ -2656,7 +2656,7 @@ bool PipelineImpl::OptimizeGraphForMidTier(Linkage* linkage) {
   // Perform simplified lowering. This has to run w/o the Typer decorator,
   // because we cannot compute meaningful types anyways, and the computed types
   // might even conflict with the representation/truncation logic.
-  Run<SimplifiedLoweringPhase>();
+  Run<SimplifiedLoweringPhase>(linkage);
   RunPrintAndVerify(SimplifiedLoweringPhase::phase_name(), true);
 
   // From now on it is invalid to look at types on the nodes, because the types

src/compiler/simplified-lowering.cc

@@ -293,7 +293,7 @@ class RepresentationSelector {
                          RepresentationChanger* changer,
                          SourcePositionTable* source_positions,
                          NodeOriginTable* node_origins,
-                         TickCounter* tick_counter)
+                         TickCounter* tick_counter, Linkage* linkage)
       : jsgraph_(jsgraph),
         zone_(zone),
         might_need_revisit_(zone),
@@ -310,7 +310,8 @@ class RepresentationSelector {
         node_origins_(node_origins),
         type_cache_(TypeCache::Get()),
         op_typer_(broker, graph_zone()),
-        tick_counter_(tick_counter) {
+        tick_counter_(tick_counter),
+        linkage_(linkage) {
   }
 
   void ResetNodeInfoState() {
@@ -1838,9 +1839,10 @@ class RepresentationSelector {
         // here, otherwise the input conversion will fail.
         return VisitLeaf<T>(node, MachineRepresentation::kTagged);
       case IrOpcode::kParameter:
-        // TODO(titzer): use representation from linkage.
         return VisitUnop<T>(node, UseInfo::None(),
-                            MachineRepresentation::kTagged);
+                            linkage()
+                                ->GetParameterType(ParameterIndexOf(node->op()))
+                                .representation());
       case IrOpcode::kInt32Constant:
         return VisitLeaf<T>(node, MachineRepresentation::kWord32);
       case IrOpcode::kInt64Constant:
@@ -2828,7 +2830,16 @@ class RepresentationSelector {
         return VisitUnop<T>(node, UseInfo::AnyTagged(),
                             MachineRepresentation::kTaggedPointer);
       }
-      case IrOpcode::kTierUpCheck:
+      case IrOpcode::kTierUpCheck: {
+        ProcessInput<T>(node, 0, UseInfo::AnyTagged());
+        ProcessInput<T>(node, 1, UseInfo::AnyTagged());
+        ProcessInput<T>(node, 2, UseInfo::AnyTagged());
+        ProcessInput<T>(node, 3, UseInfo::TruncatingWord32());
+        ProcessInput<T>(node, 4, UseInfo::AnyTagged());
+        ProcessRemainingInputs<T>(node, 5);
+        SetOutput<T>(node, MachineRepresentation::kNone);
+        return;
+      }
       case IrOpcode::kUpdateInterruptBudget: {
         ProcessInput<T>(node, 0, UseInfo::AnyTagged());
         ProcessRemainingInputs<T>(node, 1);
@@ -3836,6 +3847,7 @@ class RepresentationSelector {
   TypeCache const* type_cache_;
   OperationTyper op_typer_;  // helper for the feedback typer
   TickCounter* const tick_counter_;
+  Linkage* const linkage_;
 
   NodeInfo* GetInfo(Node* node) {
     DCHECK(node->id() < count_);
@@ -3843,6 +3855,7 @@ class RepresentationSelector {
   }
   Zone* zone() { return zone_; }
   Zone* graph_zone() { return jsgraph_->zone(); }
+  Linkage* linkage() { return linkage_; }
 };
 
 // Template specializations
@@ -4006,7 +4019,8 @@ SimplifiedLowering::SimplifiedLowering(JSGraph* jsgraph, JSHeapBroker* broker,
                                        SourcePositionTable* source_positions,
                                        NodeOriginTable* node_origins,
                                        PoisoningMitigationLevel poisoning_level,
-                                       TickCounter* tick_counter)
+                                       TickCounter* tick_counter,
+                                       Linkage* linkage)
     : jsgraph_(jsgraph),
       broker_(broker),
       zone_(zone),
@@ -4014,13 +4028,14 @@ SimplifiedLowering::SimplifiedLowering(JSGraph* jsgraph, JSHeapBroker* broker,
       source_positions_(source_positions),
       node_origins_(node_origins),
       poisoning_level_(poisoning_level),
-      tick_counter_(tick_counter) {}
+      tick_counter_(tick_counter),
+      linkage_(linkage) {}
 
 void SimplifiedLowering::LowerAllNodes() {
   RepresentationChanger changer(jsgraph(), broker_);
   RepresentationSelector selector(jsgraph(), broker_, zone_, &changer,
                                   source_positions_, node_origins_,
-                                  tick_counter_);
+                                  tick_counter_, linkage_);
   selector.Run(this);
 }
 

src/compiler/simplified-lowering.h

@@ -30,7 +30,7 @@ class V8_EXPORT_PRIVATE SimplifiedLowering final {
                      SourcePositionTable* source_position,
                      NodeOriginTable* node_origins,
                      PoisoningMitigationLevel poisoning_level,
-                     TickCounter* tick_counter);
+                     TickCounter* tick_counter, Linkage* linkage);
   ~SimplifiedLowering() = default;
 
   void LowerAllNodes();
@@ -72,6 +72,7 @@ class V8_EXPORT_PRIVATE SimplifiedLowering final {
   PoisoningMitigationLevel poisoning_level_;
 
   TickCounter* const tick_counter_;
+  Linkage* const linkage_;
 
   Node* Float64Round(Node* const node);
   Node* Float64Sign(Node* const node);
@@ -98,6 +99,7 @@ class V8_EXPORT_PRIVATE SimplifiedLowering final {
   CommonOperatorBuilder* common() { return jsgraph()->common(); }
   MachineOperatorBuilder* machine() { return jsgraph()->machine(); }
   SimplifiedOperatorBuilder* simplified() { return jsgraph()->simplified(); }
+  Linkage* linkage() { return linkage_; }
 };
 
 }  // namespace compiler

src/compiler/simplified-operator.cc

@@ -1325,7 +1325,7 @@ const Operator* SimplifiedOperatorBuilder::UpdateInterruptBudget(int delta) {
 const Operator* SimplifiedOperatorBuilder::TierUpCheck() {
   return zone()->New<Operator>(IrOpcode::kTierUpCheck,
                                Operator::kNoThrow | Operator::kNoDeopt,
-                               "TierUpCheck", 1, 1, 1, 0, 1, 0);
+                               "TierUpCheck", 5, 1, 1, 0, 1, 0);
 }
 
 const Operator* SimplifiedOperatorBuilder::AssertType(Type type) {

src/compiler/simplified-operator.h

@@ -1190,7 +1190,12 @@ class TierUpCheckNode final : public SimplifiedNodeWrapperBase {
     CONSTEXPR_DCHECK(node->opcode() == IrOpcode::kTierUpCheck);
   }
 
-#define INPUTS(V) V(FeedbackVector, feedback_vector, 0, FeedbackVector)
+#define INPUTS(V)                                       \
+  V(FeedbackVector, feedback_vector, 0, FeedbackVector) \
+  V(Target, target, 1, JSReceiver)                      \
+  V(NewTarget, new_target, 2, Object)                   \
+  V(InputCount, input_count, 3, UntaggedT)              \
+  V(Context, context, 4, Context)
   INPUTS(DEFINE_INPUT_ACCESSORS)
 #undef INPUTS
 };

test/unittests/compiler/simplified-lowering-unittest.cc

@@ -47,9 +47,11 @@ class SimplifiedLoweringTest : public GraphTest {
       typer.Run();
     }
 
+    Linkage* linkage = zone()->New<Linkage>(Linkage::GetJSCallDescriptor(
+        zone(), false, num_parameters_ + 1, CallDescriptor::kCanUseRoots));
     SimplifiedLowering lowering(
         jsgraph(), broker(), zone(), source_positions(), node_origins(),
-        PoisoningMitigationLevel::kDontPoison, tick_counter());
+        PoisoningMitigationLevel::kDontPoison, tick_counter(), linkage);
     lowering.LowerAllNodes();
   }