[turbofan][wasm][arm64] Improved saturated conversions float32 to int32.

The design of this change was discussed here:
https://docs.google.com/document/d/12otOj6SyXMXj0Dnnx9B6MGLMRwHPhg6RIZRazVw3tFA/

Bug: v8:10720
Change-Id: I8292dcf7272bdf4526a2d630b49fc374cdb01bdc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2304570
Commit-Queue: Richard Stotz <rstz@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68994}

src/compiler/backend/arm64/code-generator-arm64.cc

@@ -1518,24 +1518,32 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
     case kArm64Float64ToFloat32:
       __ Fcvt(i.OutputDoubleRegister().S(), i.InputDoubleRegister(0));
       break;
-    case kArm64Float32ToInt32:
+    case kArm64Float32ToInt32: {
       __ Fcvtzs(i.OutputRegister32(), i.InputFloat32Register(0));
-      // Avoid INT32_MAX as an overflow indicator and use INT32_MIN instead,
-      // because INT32_MIN allows easier out-of-bounds detection.
-      __ Cmn(i.OutputRegister32(), 1);
-      __ Csinc(i.OutputRegister32(), i.OutputRegister32(), i.OutputRegister32(),
-               vc);
+      bool set_overflow_to_min_i32 = MiscField::decode(instr->opcode());
+      if (set_overflow_to_min_i32) {
+        // Avoid INT32_MAX as an overflow indicator and use INT32_MIN instead,
+        // because INT32_MIN allows easier out-of-bounds detection.
+        __ Cmn(i.OutputRegister32(), 1);
+        __ Csinc(i.OutputRegister32(), i.OutputRegister32(),
+                 i.OutputRegister32(), vc);
+      }
       break;
+    }
     case kArm64Float64ToInt32:
       __ Fcvtzs(i.OutputRegister32(), i.InputDoubleRegister(0));
       break;
-    case kArm64Float32ToUint32:
+    case kArm64Float32ToUint32: {
       __ Fcvtzu(i.OutputRegister32(), i.InputFloat32Register(0));
-      // Avoid UINT32_MAX as an overflow indicator and use 0 instead,
-      // because 0 allows easier out-of-bounds detection.
-      __ Cmn(i.OutputRegister32(), 1);
-      __ Adc(i.OutputRegister32(), i.OutputRegister32(), Operand(0));
+      bool set_overflow_to_min_u32 = MiscField::decode(instr->opcode());
+      if (set_overflow_to_min_u32) {
+        // Avoid UINT32_MAX as an overflow indicator and use 0 instead,
+        // because 0 allows easier out-of-bounds detection.
+        __ Cmn(i.OutputRegister32(), 1);
+        __ Adc(i.OutputRegister32(), i.OutputRegister32(), Operand(0));
+      }
       break;
+    }
     case kArm64Float64ToUint32:
       __ Fcvtzu(i.OutputRegister32(), i.InputDoubleRegister(0));
       break;

src/compiler/backend/arm64/instruction-selector-arm64.cc

@@ -1336,10 +1336,8 @@ void InstructionSelector::VisitWord64Ror(Node* node) {
   V(ChangeInt32ToFloat64, kArm64Int32ToFloat64)               \
   V(ChangeInt64ToFloat64, kArm64Int64ToFloat64)               \
   V(ChangeUint32ToFloat64, kArm64Uint32ToFloat64)             \
-  V(TruncateFloat32ToInt32, kArm64Float32ToInt32)             \
   V(ChangeFloat64ToInt32, kArm64Float64ToInt32)               \
   V(ChangeFloat64ToInt64, kArm64Float64ToInt64)               \
-  V(TruncateFloat32ToUint32, kArm64Float32ToUint32)           \
   V(ChangeFloat64ToUint32, kArm64Float64ToUint32)             \
   V(ChangeFloat64ToUint64, kArm64Float64ToUint64)             \
   V(TruncateFloat64ToInt64, kArm64Float64ToInt64)             \
@@ -1640,6 +1638,28 @@ void InstructionSelector::VisitUint32MulHigh(Node* node) {
   Emit(kArm64Lsr, g.DefineAsRegister(node), smull_operand, g.TempImmediate(32));
 }
 
+void InstructionSelector::VisitTruncateFloat32ToInt32(Node* node) {
+  Arm64OperandGenerator g(this);
+
+  InstructionCode opcode = kArm64Float32ToInt32;
+  TruncateKind kind = OpParameter<TruncateKind>(node->op());
+  opcode |= MiscField::encode(kind == TruncateKind::kSetOverflowToMin);
+
+  Emit(opcode, g.DefineAsRegister(node), g.UseRegister(node->InputAt(0)));
+}
+
+void InstructionSelector::VisitTruncateFloat32ToUint32(Node* node) {
+  Arm64OperandGenerator g(this);
+
+  InstructionCode opcode = kArm64Float32ToUint32;
+  TruncateKind kind = OpParameter<TruncateKind>(node->op());
+  if (kind == TruncateKind::kSetOverflowToMin) {
+    opcode |= MiscField::encode(true);
+  }
+
+  Emit(opcode, g.DefineAsRegister(node), g.UseRegister(node->InputAt(0)));
+}
+
 void InstructionSelector::VisitTryTruncateFloat32ToInt64(Node* node) {
   Arm64OperandGenerator g(this);
 
@@ -3694,7 +3714,8 @@ InstructionSelector::SupportedMachineOperatorFlags() {
          MachineOperatorBuilder::kInt32DivIsSafe |
          MachineOperatorBuilder::kUint32DivIsSafe |
          MachineOperatorBuilder::kWord32ReverseBits |
-         MachineOperatorBuilder::kWord64ReverseBits;
+         MachineOperatorBuilder::kWord64ReverseBits |
+         MachineOperatorBuilder::kSatConversionIsSafe;
 }
 
 // static

src/compiler/code-assembler.cc

@@ -619,6 +619,11 @@ TNode<Float64T> CodeAssembler::RoundIntPtrToFloat64(Node* value) {
   return UncheckedCast<Float64T>(raw_assembler()->ChangeInt32ToFloat64(value));
 }
 
+TNode<Int32T> CodeAssembler::TruncateFloat32ToInt32(
+    SloppyTNode<Float32T> value) {
+  return UncheckedCast<Int32T>(raw_assembler()->TruncateFloat32ToInt32(
+      value, TruncateKind::kSetOverflowToMin));
+}
 #define DEFINE_CODE_ASSEMBLER_UNARY_OP(name, ResType, ArgType) \
   TNode<ResType> CodeAssembler::name(SloppyTNode<ArgType> a) { \
     return UncheckedCast<ResType>(raw_assembler()->name(a));   \

src/compiler/code-assembler.h

@@ -321,7 +321,6 @@ TNode<Float64T> Float64Add(TNode<Float64T> a, TNode<Float64T> b);
   V(BitcastMaybeObjectToWord, IntPtrT, MaybeObject)            \
   V(BitcastWordToTagged, Object, WordT)                        \
   V(BitcastWordToTaggedSigned, Smi, WordT)                     \
-  V(TruncateFloat32ToInt32, Int32T, Float32T)                  \
   V(TruncateFloat64ToFloat32, Float32T, Float64T)              \
   V(TruncateFloat64ToWord32, Uint32T, Float64T)                \
   V(TruncateInt64ToInt32, Int32T, Int64T)                      \
@@ -936,6 +935,12 @@ class V8_EXPORT_PRIVATE CodeAssembler {
   // No-op on 32-bit, otherwise sign extend.
   TNode<IntPtrT> ChangeInt32ToIntPtr(TNode<Word32T> value);
 
+  // Truncates a float to a 32-bit integer. If the float is outside of 32-bit
+  // range, make sure that overflow detection is easy. In particular, return
+  // int_min instead of int_max on arm platforms by using parameter
+  // kSetOverflowToMin.
+  TNode<Int32T> TruncateFloat32ToInt32(SloppyTNode<Float32T> value);
+
   // No-op that guarantees that the value is kept alive till this point even
   // if GC happens.
   Node* Retain(Node* value);

src/compiler/machine-operator.cc

@@ -235,8 +235,6 @@ ShiftKind ShiftKindOf(Operator const* op) {
   V(ChangeFloat64ToUint64, Operator::kNoProperties, 1, 0, 1)               \
   V(TruncateFloat64ToInt64, Operator::kNoProperties, 1, 0, 1)              \
   V(TruncateFloat64ToUint32, Operator::kNoProperties, 1, 0, 1)             \
-  V(TruncateFloat32ToInt32, Operator::kNoProperties, 1, 0, 1)              \
-  V(TruncateFloat32ToUint32, Operator::kNoProperties, 1, 0, 1)             \
   V(TryTruncateFloat32ToInt64, Operator::kNoProperties, 1, 0, 2)           \
   V(TryTruncateFloat64ToInt64, Operator::kNoProperties, 1, 0, 2)           \
   V(TryTruncateFloat32ToUint64, Operator::kNoProperties, 1, 0, 2)          \
@@ -1014,6 +1012,55 @@ const Operator* MachineOperatorBuilder::UnalignedStore(
   UNREACHABLE();
 }
 
+template <TruncateKind kind>
+struct TruncateFloat32ToUint32Operator : Operator1<TruncateKind> {
+  TruncateFloat32ToUint32Operator()
+      : Operator1(IrOpcode::kTruncateFloat32ToUint32, Operator::kPure,
+                  "TruncateFloat32ToUint32", 1, 0, 0, 1, 0, 0, kind) {}
+};
+
+const Operator* MachineOperatorBuilder::TruncateFloat32ToUint32(
+    TruncateKind kind) {
+  switch (kind) {
+    case TruncateKind::kArchitectureDefault:
+      return GetCachedOperator<TruncateFloat32ToUint32Operator<
+          TruncateKind::kArchitectureDefault>>();
+    case TruncateKind::kSetOverflowToMin:
+      return GetCachedOperator<
+          TruncateFloat32ToUint32Operator<TruncateKind::kSetOverflowToMin>>();
+  }
+}
+
+template <TruncateKind kind>
+struct TruncateFloat32ToInt32Operator : Operator1<TruncateKind> {
+  TruncateFloat32ToInt32Operator()
+      : Operator1(IrOpcode::kTruncateFloat32ToInt32, Operator::kPure,
+                  "TruncateFloat32ToInt32", 1, 0, 0, 1, 0, 0, kind) {}
+};
+
+const Operator* MachineOperatorBuilder::TruncateFloat32ToInt32(
+    TruncateKind kind) {
+  switch (kind) {
+    case TruncateKind::kArchitectureDefault:
+      return GetCachedOperator<
+          TruncateFloat32ToInt32Operator<TruncateKind::kArchitectureDefault>>();
+    case TruncateKind::kSetOverflowToMin:
+      return GetCachedOperator<
+          TruncateFloat32ToInt32Operator<TruncateKind::kSetOverflowToMin>>();
+  }
+}
+
+size_t hash_value(TruncateKind kind) { return static_cast<size_t>(kind); }
+
+std::ostream& operator<<(std::ostream& os, TruncateKind kind) {
+  switch (kind) {
+    case TruncateKind::kArchitectureDefault:
+      return os << "kArchitectureDefault";
+    case TruncateKind::kSetOverflowToMin:
+      return os << "kSetOverflowToMin";
+  }
+}
+
 #define PURE(Name, properties, value_input_count, control_input_count,     \
              output_count)                                                 \
   const Operator* MachineOperatorBuilder::Name() {                         \

src/compiler/machine-operator.h

@@ -192,6 +192,12 @@ size_t hash_value(ShiftKind);
 V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream&, ShiftKind);
 ShiftKind ShiftKindOf(Operator const*) V8_WARN_UNUSED_RESULT;
 
+// TruncateKind::kSetOverflowToMin sets the result of a saturating float-to-int
+// conversion to INT_MIN if the conversion returns INT_MAX due to overflow. This
+// makes it easier to detect an overflow. This parameter is ignored on platforms
+// like x64 and ia32 where a range overflow does not result in INT_MAX.
+enum class TruncateKind { kArchitectureDefault, kSetOverflowToMin };
+
 // Interface for building machine-level operators. These operators are
 // machine-level but machine-independent and thus define a language suitable
 // for generating code to run on architectures such as ia32, x64, arm, etc.
@@ -224,13 +230,14 @@ class V8_EXPORT_PRIVATE MachineOperatorBuilder final
     kInt64AbsWithOverflow = 1u << 21,
     kWord32Rol = 1u << 22,
     kWord64Rol = 1u << 23,
+    kSatConversionIsSafe = 1u << 24,
     kAllOptionalOps =
         kFloat32RoundDown | kFloat64RoundDown | kFloat32RoundUp |
         kFloat64RoundUp | kFloat32RoundTruncate | kFloat64RoundTruncate |
         kFloat64RoundTiesAway | kFloat32RoundTiesEven | kFloat64RoundTiesEven |
         kWord32Ctz | kWord64Ctz | kWord32Popcnt | kWord64Popcnt |
         kWord32ReverseBits | kWord64ReverseBits | kInt32AbsWithOverflow |
-        kInt64AbsWithOverflow | kWord32Rol | kWord64Rol
+        kInt64AbsWithOverflow | kWord32Rol | kWord64Rol | kSatConversionIsSafe
   };
   using Flags = base::Flags<Flag, unsigned>;
 
@@ -332,6 +339,11 @@ class V8_EXPORT_PRIVATE MachineOperatorBuilder final
   // generate a mask with 0x1f on the amount ahead of generating the shift.
   bool Word32ShiftIsSafe() const { return flags_ & kWord32ShiftIsSafe; }
 
+  // Return true if the target's implementation of float-to-int-conversions is a
+  // saturating conversion rounding towards 0. Otherwise, we have to manually
+  // generate the correct value if a saturating conversion is requested.
+  bool SatConversionIsSafe() const { return flags_ & kSatConversionIsSafe; }
+
   const Operator* Word64And();
   const Operator* Word64Or();
   const Operator* Word64Xor();
@@ -417,15 +429,19 @@ class V8_EXPORT_PRIVATE MachineOperatorBuilder final
   // in the target type and are *not* defined for other inputs.
   // Use narrowing change operators only when there is a static guarantee that
   // the input value is representable in the target value.
+  //
+  // Some operators can have the behaviour on overflow change through specifying
+  // TruncateKind. The exact semantics are documented in the tests in
+  // test/cctest/compiler/test-run-machops.cc .
   const Operator* ChangeFloat32ToFloat64();
   const Operator* ChangeFloat64ToInt32();   // narrowing
   const Operator* ChangeFloat64ToInt64();
   const Operator* ChangeFloat64ToUint32();  // narrowing
   const Operator* ChangeFloat64ToUint64();
   const Operator* TruncateFloat64ToInt64();
   const Operator* TruncateFloat64ToUint32();
-  const Operator* TruncateFloat32ToInt32();
-  const Operator* TruncateFloat32ToUint32();
+  const Operator* TruncateFloat32ToInt32(TruncateKind kind);
+  const Operator* TruncateFloat32ToUint32(TruncateKind kind);
   const Operator* TryTruncateFloat32ToInt64();
   const Operator* TryTruncateFloat64ToInt64();
   const Operator* TryTruncateFloat32ToUint64();

src/compiler/raw-machine-assembler.h

@@ -721,11 +721,11 @@ class V8_EXPORT_PRIVATE RawMachineAssembler {
   Node* TruncateFloat64ToUint32(Node* a) {
     return AddNode(machine()->TruncateFloat64ToUint32(), a);
   }
-  Node* TruncateFloat32ToInt32(Node* a) {
-    return AddNode(machine()->TruncateFloat32ToInt32(), a);
+  Node* TruncateFloat32ToInt32(Node* a, TruncateKind kind) {
+    return AddNode(machine()->TruncateFloat32ToInt32(kind), a);
   }
-  Node* TruncateFloat32ToUint32(Node* a) {
-    return AddNode(machine()->TruncateFloat32ToUint32(), a);
+  Node* TruncateFloat32ToUint32(Node* a, TruncateKind kind) {
+    return AddNode(machine()->TruncateFloat32ToUint32(kind), a);
   }
   Node* TryTruncateFloat32ToInt64(Node* a) {
     return AddNode(machine()->TryTruncateFloat32ToInt64(), a);

src/compiler/wasm-compiler.cc

@@ -1550,11 +1550,17 @@ MachineType FloatConvertType(wasm::WasmOpcode opcode) {
 const Operator* ConvertOp(WasmGraphBuilder* builder, wasm::WasmOpcode opcode) {
   switch (opcode) {
     case wasm::kExprI32SConvertF32:
+      return builder->mcgraph()->machine()->TruncateFloat32ToInt32(
+          TruncateKind::kSetOverflowToMin);
     case wasm::kExprI32SConvertSatF32:
-      return builder->mcgraph()->machine()->TruncateFloat32ToInt32();
+      return builder->mcgraph()->machine()->TruncateFloat32ToInt32(
+          TruncateKind::kArchitectureDefault);
     case wasm::kExprI32UConvertF32:
+      return builder->mcgraph()->machine()->TruncateFloat32ToUint32(
+          TruncateKind::kSetOverflowToMin);
     case wasm::kExprI32UConvertSatF32:
-      return builder->mcgraph()->machine()->TruncateFloat32ToUint32();
+      return builder->mcgraph()->machine()->TruncateFloat32ToUint32(
+          TruncateKind::kArchitectureDefault);
     case wasm::kExprI32SConvertF64:
     case wasm::kExprI32SConvertSatF64:
       return builder->mcgraph()->machine()->ChangeFloat64ToInt32();
@@ -1753,6 +1759,9 @@ Node* WasmGraphBuilder::BuildIntConvertFloat(Node* input,
     }
     return converted_value;
   }
+  if (mcgraph()->machine()->SatConversionIsSafe()) {
+    return converted_value;
+  }
   Node* test = ConvertSaturateTest(this, opcode, int_ty, float_ty, trunc,
                                    converted_value);
   Diamond tl_d(graph(), mcgraph()->common(), test, BranchHint::kFalse);

test/cctest/compiler/test-multiple-return.cc

@@ -110,7 +110,7 @@ Node* ToInt32(RawMachineAssembler* m, MachineType type, Node* a) {
     case MachineRepresentation::kWord64:
       return m->TruncateInt64ToInt32(a);
     case MachineRepresentation::kFloat32:
-      return m->TruncateFloat32ToInt32(a);
+      return m->TruncateFloat32ToInt32(a, TruncateKind::kArchitectureDefault);
     case MachineRepresentation::kFloat64:
       return m->RoundFloat64ToInt32(a);
     default: