[Turbofan] Improve verification with effect output checks

Ensure that effect outputs from nodes are used if they have
an effect output. This helps us avoid an easy-to-make error
where we fail to update the effect chain with the result of
effectful operations.

Bug: v8:6929
Change-Id: I585dc627b3c330006ec04717ff9b2f5060dbad6a
Reviewed-on: https://chromium-review.googlesource.com/718107
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48861}

src/compiler/pipeline.cc

@@ -1743,8 +1743,21 @@ struct VerifyGraphPhase {
 
   void Run(PipelineData* data, Zone* temp_zone, const bool untyped,
            bool values_only = false) {
+    Verifier::CodeType code_type;
+    switch (data->info()->code_kind()) {
+      case Code::WASM_FUNCTION:
+      case Code::WASM_TO_JS_FUNCTION:
+      case Code::JS_TO_WASM_FUNCTION:
+      case Code::WASM_INTERPRETER_ENTRY:
+      case Code::C_WASM_ENTRY:
+        code_type = Verifier::kWasm;
+        break;
+      default:
+        code_type = Verifier::kDefault;
+    }
     Verifier::Run(data->graph(), !untyped ? Verifier::TYPED : Verifier::UNTYPED,
-                  values_only ? Verifier::kValuesOnly : Verifier::kAll);
+                  values_only ? Verifier::kValuesOnly : Verifier::kAll,
+                  code_type);
   }
 };
 

src/compiler/verifier.cc

@@ -32,14 +32,18 @@ namespace compiler {
 
 class Verifier::Visitor {
  public:
-  Visitor(Zone* z, Typing typed, CheckInputs check_inputs)
-      : zone(z), typing(typed), check_inputs(check_inputs) {}
+  Visitor(Zone* z, Typing typed, CheckInputs check_inputs, CodeType code_type)
+      : zone(z),
+        typing(typed),
+        check_inputs(check_inputs),
+        code_type(code_type) {}
 
   void Check(Node* node);
 
   Zone* zone;
   Typing typing;
   CheckInputs check_inputs;
+  CodeType code_type;
 
  private:
   void CheckNotTyped(Node* node) {
@@ -112,6 +116,20 @@ void Verifier::Visitor::Check(Node* node) {
   }
   CHECK_EQ(input_count, node->InputCount());
 
+  // If this node has any effect outputs, make sure that it is
+  // consumed as an effect input somewhere else.
+  // TODO(mvstanton): support this kind of verification for WASM
+  // compiles, too.
+  if (code_type != kWasm && node->op()->EffectOutputCount() > 0) {
+    int effect_edges = 0;
+    for (Edge edge : node->use_edges()) {
+      if (NodeProperties::IsEffectEdge(edge)) {
+        effect_edges++;
+      }
+    }
+    DCHECK_GT(effect_edges, 0);
+  }
+
   // Verify that frame state has been inserted for the nodes that need it.
   for (int i = 0; i < frame_state_count; i++) {
     Node* frame_state = NodeProperties::GetFrameStateInput(node);
@@ -1534,11 +1552,12 @@ void Verifier::Visitor::Check(Node* node) {
   }
 }  // NOLINT(readability/fn_size)
 
-void Verifier::Run(Graph* graph, Typing typing, CheckInputs check_inputs) {
+void Verifier::Run(Graph* graph, Typing typing, CheckInputs check_inputs,
+                   CodeType code_type) {
   CHECK_NOT_NULL(graph->start());
   CHECK_NOT_NULL(graph->end());
   Zone zone(graph->zone()->allocator(), ZONE_NAME);
-  Visitor visitor(&zone, typing, check_inputs);
+  Visitor visitor(&zone, typing, check_inputs, code_type);
   AllNodes all(&zone, graph);
   for (Node* node : all.reachable) visitor.Check(node);
 
@@ -1822,6 +1841,7 @@ void Verifier::VerifyNode(Node* node) {
       }
     }
   }
+
   // Frame state input should be a frame state (or sentinel).
   if (OperatorProperties::GetFrameStateInputCount(node->op()) > 0) {
     Node* input = NodeProperties::GetFrameStateInput(node);

src/compiler/verifier.h

@@ -23,9 +23,11 @@ class Verifier {
  public:
   enum Typing { TYPED, UNTYPED };
   enum CheckInputs { kValuesOnly, kAll };
+  enum CodeType { kDefault, kWasm };
 
   static void Run(Graph* graph, Typing typing = TYPED,
-                  CheckInputs check_inputs = kAll);
+                  CheckInputs check_inputs = kAll,
+                  CodeType code_type = kDefault);
 
 #ifdef DEBUG
   // Verifies consistency of node inputs and uses:

test/cctest/wasm/wasm-run-utils.cc

@@ -353,7 +353,7 @@ Handle<Code> WasmFunctionWrapper::GetWrapperCode() {
     }
 
     CompilationInfo info(ArrayVector("testing"), isolate, graph()->zone(),
-                         Code::STUB);
+                         Code::WASM_FUNCTION);
     code_ = compiler::Pipeline::GenerateCodeForTesting(&info, descriptor,
                                                        graph(), nullptr);
     CHECK(!code_.is_null());