[wasm] Use GraphAssembler in wrappers

This moves first parts of the wrapper generation to the GraphAssembler.
We should migrate more code in follow-up CLs, and think about also
computing the schedule in the GraphAssembler (once everything is
migrated).

This also removes the only uses of the controversial {HalfDiamond}
construct, hence this is also removed in this CL.
Plus a bug fix in the GraphAssembler::Call method, and a new method
in GraphAssembler to load heap number values.

R=jkummerow@chromium.org, tebbi@chromium.org

Bug: v8:10123
Change-Id: Iac4661cdd50049cb73a2f305e280c1af6200729a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2000756Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65784}

src/compiler/diamond.h

@@ -13,42 +13,30 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
-struct Diamond;
-
-// A helper to make it easier to build branches that are not fully
-// diamond-shaped.
-struct HalfDiamond {
+// A helper to make it easier to build diamond-shaped control patterns.
+struct Diamond {
   Graph* graph;
   CommonOperatorBuilder* common;
   Node* branch;
   Node* if_true;
   Node* if_false;
+  Node* merge;
 
-  HalfDiamond(Graph* g, CommonOperatorBuilder* b, Node* cond,
-              BranchHint hint = BranchHint::kNone) {
+  Diamond(Graph* g, CommonOperatorBuilder* b, Node* cond,
+          BranchHint hint = BranchHint::kNone) {
     graph = g;
     common = b;
     branch = graph->NewNode(common->Branch(hint), cond, graph->start());
     if_true = graph->NewNode(common->IfTrue(), branch);
     if_false = graph->NewNode(common->IfFalse(), branch);
+    merge = graph->NewNode(common->Merge(2), if_true, if_false);
   }
 
   // Place {this} after {that} in control flow order.
-  void Chain(Node* that) { branch->ReplaceInput(1, that); }
+  void Chain(Diamond const& that) { branch->ReplaceInput(1, that.merge); }
 
   // Place {this} after {that} in control flow order.
-  inline void Chain(Diamond const& that);
-};
-
-// A helper to make it easier to build diamond-shaped control patterns.
-struct Diamond : public HalfDiamond {
-  Node* merge;
-
-  Diamond(Graph* g, CommonOperatorBuilder* b, Node* cond,
-          BranchHint hint = BranchHint::kNone)
-      : HalfDiamond(g, b, cond, hint) {
-    merge = graph->NewNode(common->Merge(2), if_true, if_false);
-  }
+  void Chain(Node* that) { branch->ReplaceInput(1, that); }
 
   // Nest {this} into either the if_true or if_false branch of {that}.
   void Nest(Diamond const& that, bool if_true) {
@@ -70,10 +58,6 @@ struct Diamond : public HalfDiamond {
   }
 };
 
-void HalfDiamond::Chain(Diamond const& that) {
-  branch->ReplaceInput(1, that.merge);
-}
-
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/graph-assembler.cc

@@ -404,6 +404,11 @@ Node* GraphAssembler::LoadFramePointer() {
   return AddNode(graph()->NewNode(machine()->LoadFramePointer()));
 }
 
+Node* GraphAssembler::LoadHeapNumberValue(Node* heap_number) {
+  return Load(MachineType::Float64(), heap_number,
+              IntPtrConstant(HeapNumber::kValueOffset - kHeapObjectTag));
+}
+
 #define SINGLETON_CONST_DEF(Name, Type)              \
   TNode<Type> JSGraphAssembler::Name##Constant() {   \
     return TNode<Type>::UncheckedCast(               \
@@ -656,9 +661,8 @@ Node* GraphAssembler::Store(StoreRepresentation rep, Node* object, Node* offset,
 }
 
 Node* GraphAssembler::Load(MachineType type, Node* object, Node* offset) {
-  Node* value = AddNode(graph()->NewNode(machine()->Load(type), object, offset,
-                                         effect(), control()));
-  return value;
+  return AddNode(graph()->NewNode(machine()->Load(type), object, offset,
+                                  effect(), control()));
 }
 
 Node* GraphAssembler::StoreUnaligned(MachineRepresentation rep, Node* object,

src/compiler/graph-assembler.h

@@ -279,6 +279,8 @@ class V8_EXPORT_PRIVATE GraphAssembler {
 
   Node* LoadFramePointer();
 
+  Node* LoadHeapNumberValue(Node* heap_number);
+
 #define PURE_UNOP_DECL(Name) Node* Name(Node* input);
   PURE_ASSEMBLER_MACH_UNOP_LIST(PURE_UNOP_DECL)
 #undef PURE_UNOP_DECL
@@ -650,10 +652,7 @@ Node* GraphAssembler::Call(const Operator* op, Args... args) {
   Node* args_array[] = {args..., effect(), control()};
   int size = static_cast<int>(sizeof...(args)) + op->EffectInputCount() +
              op->ControlInputCount();
-  Node* call = graph()->NewNode(op, size, args_array);
-  DCHECK_EQ(0, op->ControlOutputCount());
-  effect_ = call;
-  return AddNode(call);
+  return AddNode(graph()->NewNode(op, size, args_array));
 }
 
 class V8_EXPORT_PRIVATE JSGraphAssembler : public GraphAssembler {

src/compiler/wasm-compiler.cc

@@ -23,6 +23,7 @@
 #include "src/compiler/common-operator.h"
 #include "src/compiler/compiler-source-position-table.h"
 #include "src/compiler/diamond.h"
+#include "src/compiler/graph-assembler.h"
 #include "src/compiler/graph-visualizer.h"
 #include "src/compiler/graph.h"
 #include "src/compiler/int64-lowering.h"
@@ -5323,24 +5324,6 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
     return pos;
   }
 
-  // Calls the ECMAScript ToNumber operation on {node}. Returns a {Smi} or
-  // {HeapNumber}.
-  Node* BuildJavaScriptToNumber(Node* node, Node* js_context) {
-    auto call_descriptor = Linkage::GetStubCallDescriptor(
-        mcgraph()->zone(), TypeConversionDescriptor{}, 0,
-        CallDescriptor::kNoFlags, Operator::kNoProperties, stub_mode_);
-    Node* target =
-        GetTargetForBuiltinCall(wasm::WasmCode::kToNumber, Builtins::kToNumber);
-
-    Node* result = SetEffect(
-        graph()->NewNode(mcgraph()->common()->Call(call_descriptor), target,
-                         node, js_context, Effect(), Control()));
-
-    SetSourcePosition(result, 1);
-
-    return result;
-  }
-
   // Converts a number (Smi or HeapNumber) to float64.
   Node* BuildChangeNumberToFloat64(Node* value) {
     // If the input is a HeapNumber, we load the value from it.
@@ -5505,10 +5488,13 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
         break;
     }
 
+    // TODO(wasm): Use GraphAssembler more widely.
+    GraphAssembler gasm(mcgraph(), mcgraph()->zone());
+    gasm.InitializeEffectControl(Effect(), Control());
+
     // Handle Smis first. The rest goes through the ToNumber stub.
     // TODO(clemensb): Also handle HeapNumbers specially.
     STATIC_ASSERT(kSmiTagMask < kMaxUInt32);
-    CommonOperatorBuilder* common = mcgraph()->common();
 
     // Build a graph implementing this diagram:
     //   input smi?
@@ -5517,48 +5503,44 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
     //                          ├─ true: ─┘                     │
     //                          └─ false: load -> f64-to-int32 ─┘
 
-    HalfDiamond input_is_smi(graph(), common, BuildTestSmi(input),
-                             BranchHint::kTrue);
-    input_is_smi.Chain(Control());
-    Node* effect_orig = Effect();
+    auto smi_to_int32 = gasm.MakeLabel(MachineRepresentation::kTaggedSigned);
+    auto done =
+        gasm.MakeLabel(wasm::ValueTypes::MachineRepresentationFor(type));
 
-    // If not smi, call ToNumber and check if the result is Smi.
-    SetControl(input_is_smi.if_false);
-    Node* num = BuildJavaScriptToNumber(input, js_context);
-    Node* effect_after_tonumber = Effect();
-    HalfDiamond num_is_smi(graph(), common, BuildTestSmi(num),
-                           BranchHint::kTrue);
-    num_is_smi.Chain(Control());
+    // If the input is Smi, directly convert to int32.
+    gasm.GotoIf(BuildTestSmi(input), &smi_to_int32, input);
+
+    // Otherwise, call ToNumber which returns a Smi or HeapNumber.
+    auto to_number_descriptor = Linkage::GetStubCallDescriptor(
+        mcgraph()->zone(), TypeConversionDescriptor{}, 0,
+        CallDescriptor::kNoFlags, Operator::kNoProperties, stub_mode_);
+    Node* to_number_target =
+        GetTargetForBuiltinCall(wasm::WasmCode::kToNumber, Builtins::kToNumber);
+
+    Node* number =
+        gasm.Call(to_number_descriptor, to_number_target, input, js_context);
+    SetSourcePosition(number, 1);
+
+    // If the ToNumber result is Smi, convert to int32.
+    gasm.GotoIf(BuildTestSmi(number), &smi_to_int32, number);
 
-    // If ToNumber returns HeapNumber, load its value and convert to {type}.
-    SetControl(num_is_smi.if_false);
-    Node* heap_number_value = BuildLoadHeapNumberValue(num);
+    // Otherwise the ToNumber result is a HeapNumber. Load its value and convert
+    // to {type}.
+    Node* heap_number_value = gasm.LoadHeapNumberValue(number);
     Node* converted_heap_number_value =
         BuildFloat64ToWasm(heap_number_value, type);
-    Node* effect_after_heapnumber = Effect();
-
-    // Now merge the two Smi cases together, and convert the Smi to {type}.
-    Node* smi_merge = graph()->NewNode(common->Merge(2), input_is_smi.if_true,
-                                       num_is_smi.if_true);
-    Node* smi_effect = graph()->NewNode(common->EffectPhi(2), effect_orig,
-                                        effect_after_tonumber, smi_merge);
-    Node* smi_value =
-        graph()->NewNode(common->Phi(MachineRepresentation::kTaggedSigned, 2),
-                         input, num, smi_merge);
-    Node* converted_smi = BuildSmiToWasm(smi_value, type);
-
-    // Merge the final result, control and effect.
-    Node* final_merge =
-        graph()->NewNode(common->Merge(2), smi_merge, num_is_smi.if_false);
-    Node* final_value = graph()->NewNode(
-        common->Phi(wasm::ValueTypes::MachineRepresentationFor(type), 2),
-        converted_smi, converted_heap_number_value, final_merge);
-    Node* final_effect = graph()->NewNode(common->EffectPhi(2), smi_effect,
-                                          effect_after_heapnumber, final_merge);
-
-    SetEffect(final_effect);
-    SetControl(final_merge);
-    return final_value;
+    gasm.Goto(&done, converted_heap_number_value);
+
+    // Now implement the smi to int32 conversion.
+    gasm.Bind(&smi_to_int32);
+    Node* smi_int32 = BuildSmiToWasm(smi_to_int32.PhiAt(0), type);
+    gasm.Goto(&done, smi_int32);
+
+    // Done. Update effect and control and return the final Phi.
+    gasm.Bind(&done);
+    SetEffect(gasm.effect());
+    SetControl(gasm.control());
+    return done.PhiAt(0);
   }
 
   void BuildModifyThreadInWasmFlag(bool new_value) {