[stubs] Enable graph verification for builtins.

src/compiler/machine-graph-verifier.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler/machine-graph-verifier.h"
 
 #include "src/compiler/common-operator.h"
 #include "src/compiler/graph.h"
 #include "src/compiler/linkage.h"
 #include "src/compiler/machine-operator.h"
@@ skipping 12 matching lines @@
  public:
   MachineRepresentationInferrer(Schedule const* schedule, Graph const* graph,
                                 Linkage* linkage, Zone* zone)
       : schedule_(schedule),
         linkage_(linkage),
         representation_vector_(graph->NodeCount(), MachineRepresentation::kNone,
                                zone) {
     Run();
   }
 
+  CallDescriptor* call_descriptor() const {
+    return linkage_->GetIncomingDescriptor();
+  }
+
   MachineRepresentation GetRepresentation(Node const* node) const {
     return representation_vector_.at(node->id());
   }
 
  private:
   MachineRepresentation GetProjectionType(Node const* projection) {
     size_t index = ProjectionIndexOf(projection->op());
     Node* input = projection->InputAt(0);
     switch (input->opcode()) {
       case IrOpcode::kInt32AddWithOverflow:
@@ skipping 44 matching lines @@
         if (node == nullptr) {
           DCHECK_EQ(block->NodeCount(), i);
           break;
         }
         switch (node->opcode()) {
           case IrOpcode::kParameter:
             representation_vector_[node->id()] =
                 linkage_->GetParameterType(ParameterIndexOf(node->op()))
                     .representation();
             break;
+          case IrOpcode::kReturn: {
+            representation_vector_[node->id()] = PromoteRepresentation(
+                linkage_->GetReturnType().representation());
+            break;
+          }
           case IrOpcode::kProjection: {
             representation_vector_[node->id()] = GetProjectionType(node);
           } break;
           case IrOpcode::kTypedStateValues:
             representation_vector_[node->id()] = MachineRepresentation::kNone;
             break;
           case IrOpcode::kAtomicLoad:
           case IrOpcode::kLoad:
           case IrOpcode::kProtectedLoad:
             representation_vector_[node->id()] = PromoteRepresentation(
@@ skipping 22 matching lines @@
             if (desc->ReturnCount() > 0) {
               representation_vector_[node->id()] =
                   desc->GetReturnType(0).representation();
             } else {
               representation_vector_[node->id()] =
                   MachineRepresentation::kTagged;
             }
             break;
           }
           case IrOpcode::kAtomicStore:
+            representation_vector_[node->id()] =
+                PromoteRepresentation(AtomicStoreRepresentationOf(node->op()));
+            break;
           case IrOpcode::kStore:
           case IrOpcode::kProtectedStore:
             representation_vector_[node->id()] = PromoteRepresentation(
                 StoreRepresentationOf(node->op()).representation());
             break;
           case IrOpcode::kCheckedStore:
             representation_vector_[node->id()] =
                 PromoteRepresentation(CheckedStoreRepresentationOf(node->op()));
             break;
           case IrOpcode::kUnalignedStore:
@@ skipping 300 matching lines @@
             break;
           case IrOpcode::kPhi:
             switch (inferrer_->GetRepresentation(node)) {
               case MachineRepresentation::kTagged:
               case MachineRepresentation::kTaggedPointer:
               case MachineRepresentation::kTaggedSigned:
                 for (int i = 0; i < node->op()->ValueInputCount(); ++i) {
                   CheckValueInputIsTagged(node, i);
                 }
                 break;
+              case MachineRepresentation::kWord32:
+                for (int i = 0; i < node->op()->ValueInputCount(); ++i) {
+                  CheckValueInputForInt32Op(node, i);
+                }
+                break;
               default:
                 for (int i = 0; i < node->op()->ValueInputCount(); ++i) {
                   CheckValueInputRepresentationIs(
                       node, i, inferrer_->GetRepresentation(node));
                 }
                 break;
             }
             break;
           case IrOpcode::kBranch:
           case IrOpcode::kSwitch:
             CheckValueInputForInt32Op(node, 0);
             break;
-          case IrOpcode::kReturn:
-            // TODO(epertoso): use the linkage to determine which tipe we
-            // should have here.
+          case IrOpcode::kReturn: {
+            // TODO(ishell): enable once the pop count parameter type becomes
+            // MachineType::PointerRepresentation(). Currently it's int32 or
+            // word-size.
+            // CheckValueInputRepresentationIs(
+            //     node, 0, MachineType::PointerRepresentation());  // Pop count
+            size_t return_count = inferrer_->call_descriptor()->ReturnCount();
+            for (size_t i = 0; i < return_count; i++) {
+              MachineType type = inferrer_->call_descriptor()->GetReturnType(i);
+              int input_index = static_cast<int>(i + 1);
+              switch (type.representation()) {
+                case MachineRepresentation::kTagged:
+                case MachineRepresentation::kTaggedPointer:
+                case MachineRepresentation::kTaggedSigned:
+                  CheckValueInputIsTagged(node, input_index);
+                  break;
+                case MachineRepresentation::kWord32:
+                  CheckValueInputForInt32Op(node, input_index);
+                  break;
+                default:
+                  CheckValueInputRepresentationIs(
+                      node, 2, inferrer_->GetRepresentation(node));
+              }
+              break;

[Nico, 2017/09/15 19:28:25]

This break is probably supposed to go in the default branch instead of after the
switch? As-is, the code looks like

  for (...) {
    switch (...) {}
    break;
  }

i.e. the for loop runs exactly once, with i = 0.

(found by this warning, which I'm evaluating:

../../v8/src/compiler/machine-graph-verifier.cc(535,50):  error: loop will run
at most once (loop increment never executed)
[-Werror,-Wunreachable-code-loop-increment]
            for (size_t i = 0; i < return_count; i++) {
                                                 ^~~
)

[Igor Sheludko, 2017/09/20 15:15:20]

Nice catch! Thank you!
The fix: https://chromium-review.googlesource.com/c/v8/v8/+/674940

+            }
             break;
+          }
           case IrOpcode::kTypedStateValues:
           case IrOpcode::kFrameState:
             break;
           default:
             if (node->op()->ValueInputCount() != 0) {
               std::stringstream str;
               str << "Node #" << node->id() << ":" << *node->op()
                   << " in the machine graph is not being checked.";
               FATAL(str.str().c_str());
             }
@@ skipping 253 matching lines @@
   MachineRepresentationInferrer representation_inferrer(schedule, graph,
                                                         linkage, temp_zone);
   MachineRepresentationChecker checker(schedule, &representation_inferrer,
                                        is_stub);
   checker.Run();
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8