Handle negative zeros on unary minus

Before this patch, unary minus "-value" was implemented
as "0 - value" but that is not equivalent for floats when
0.0 is given.

This patch addresses this issue by implementing unary
minus as its own operation and improves the coverage.

erts/emulator/beam/emu/arith_instrs.tab

@@ -19,6 +19,24 @@
 // %CopyrightEnd%
 //
 
+OUTLINED_ARITH_1(Fail, Name, BIF, Op1,Dst) {
+    Eterm result;
+#ifdef DEBUG
+    Eterm* orig_htop = HTOP;
+    Eterm* orig_stop = E;
+#endif
+    DEBUG_SWAPOUT;
+    result = erts_$Name (c_p, $Op1);
+    DEBUG_SWAPIN;
+    ASSERT(orig_htop == HTOP && orig_stop == E);
+    ERTS_HOLE_CHECK(c_p);
+    if (ERTS_LIKELY(is_value(result))) {
+        $Dst = result;
+        $NEXT0();
+    }
+    $BIF_ERROR_ARITY_1($Fail, $BIF, $Op1);
+}
+
 OUTLINED_ARITH_2(Fail, Name, BIF, Op1, Op2, Dst) {
     Eterm result;
 #ifdef DEBUG
@@ -136,6 +154,27 @@ minus.execute(Fail, Dst) {
     $OUTLINED_ARITH_2($Fail, mixed_minus, BIF_sminus_2, MinusOp1, MinusOp2, $Dst);
 }
 
+i_unary_minus := unary_minus.fetch.execute;
+
+unary_minus.head() {
+    Eterm MinusOp;
+}
+
+unary_minus.fetch(Op) {
+    MinusOp = $Op;
+}
+
+unary_minus.execute(Fail, Dst) {
+    if (ERTS_LIKELY(is_small(MinusOp))) {
+        Sint i = -signed_val(MinusOp);
+        if (ERTS_LIKELY(IS_SSMALL(i))) {
+            $Dst = make_small(i);
+            $NEXT0();
+        }
+    }
+    $OUTLINED_ARITH_1($Fail, unary_minus, BIF_sminus_1, MinusOp, $Dst);
+}
+
 i_increment := increment.fetch.execute;
 
 increment.head() {

erts/emulator/beam/emu/ops.tab

@@ -1503,7 +1503,7 @@ gc_bif2 Fail Live u$bif:erlang:splus/2 S1 S2 Dst => \
    gen_plus Fail Live S1 S2 Dst
 
 gc_bif1 Fail Live u$bif:erlang:sminus/1 Src Dst => \
-   gen_minus Fail Live i Src Dst
+   i_unary_minus Src Fail Dst
 gc_bif2 Fail Live u$bif:erlang:sminus/2 S1 S2 Dst => \
    gen_minus Fail Live S1 S2 Dst
 
@@ -1569,6 +1569,8 @@ i_plus S1=c S2=xy Fail Dst => i_plus S2 S1 Fail Dst
 
 i_plus xy xyc j? d
 
+i_unary_minus cxy j? d
+
 # A minus instruction with a constant right operand will be
 # converted to an i_increment instruction, except in guards or
 # when the negated value of the constant won't fit in a guard.

erts/emulator/beam/erl_arith.c

@@ -79,7 +79,7 @@ BIF_RETTYPE splus_2(BIF_ALIST_2)
 
 BIF_RETTYPE sminus_1(BIF_ALIST_1)
 {
-    BIF_RET(erts_mixed_minus(BIF_P, make_small(0), BIF_ARG_1));
+    BIF_RET(erts_unary_minus(BIF_P, BIF_ARG_1));
 } 
 
 BIF_RETTYPE sminus_2(BIF_ALIST_2)
@@ -461,6 +461,71 @@ erts_mixed_plus(Process* p, Eterm arg1, Eterm arg2)
     }
 }
 
+/*
+ * While "-value" is generally the same as "0 - value",
+ * that's not true for floats due to positive and negative
+ * zeros, so we implement unary minus as its own operation.
+ */
+Eterm
+erts_unary_minus(Process* p, Eterm arg)
+{
+    Eterm hdr, res;
+    FloatDef f;
+    dsize_t sz;
+    int need_heap;
+    Eterm* hp;
+    Sint ires;
+
+    ERTS_FP_CHECK_INIT(p);
+    switch (arg & _TAG_PRIMARY_MASK) {
+    case TAG_PRIMARY_IMMED1:
+        switch ((arg & _TAG_IMMED1_MASK) >> _TAG_PRIMARY_SIZE) {
+        case (_TAG_IMMED1_SMALL >> _TAG_PRIMARY_SIZE):
+            ires = -signed_val(arg);
+            if (IS_SSMALL(ires)) {
+                return make_small(ires);
+            } else {
+                hp = HeapFragOnlyAlloc(p, 2);
+                res = small_to_big(ires, hp);
+                return res;
+            }
+        default:
+        badarith:
+            p->freason = BADARITH;
+            return THE_NON_VALUE;
+        }
+    case TAG_PRIMARY_BOXED:
+        hdr = *boxed_val(arg);
+        switch ((hdr & _TAG_HEADER_MASK) >> _TAG_PRIMARY_SIZE) {
+        case (_TAG_HEADER_POS_BIG >> _TAG_PRIMARY_SIZE):
+        case (_TAG_HEADER_NEG_BIG >> _TAG_PRIMARY_SIZE): {
+            Eterm zero_buf[2] = {make_pos_bignum_header(1), 0};
+            Eterm zero = make_big(zero_buf);
+            sz = big_size(arg);
+            need_heap = BIG_NEED_SIZE(sz);
+            hp = HeapFragOnlyAlloc(p, need_heap);
+            res = big_minus(zero, arg, hp);
+            maybe_shrink(p, hp, res, need_heap);
+            ASSERT(is_not_nil(res));
+            return res;
+        }
+        case (_TAG_HEADER_FLOAT >> _TAG_PRIMARY_SIZE):
+            GET_DOUBLE(arg, f);
+            f.fd = -f.fd;
+            ERTS_FP_ERROR(p, f.fd, goto badarith);
+            hp = HeapFragOnlyAlloc(p, FLOAT_SIZE_OBJECT);
+            res = make_float(hp);
+            PUT_DOUBLE(f, hp);
+            return res;
+        default:
+            goto badarith;
+        }
+    default:
+        goto badarith;
+    }
+}
+
+
 Eterm
 erts_mixed_minus(Process* p, Eterm arg1, Eterm arg2)
 {

erts/emulator/beam/global.h

@@ -1448,6 +1448,7 @@ Eterm collect_memory(Process *);
 void dump_memory_to_fd(int);
 int dump_memory_data(const char *);
 
+Eterm erts_unary_minus(Process* p, Eterm arg1);
 Eterm erts_mixed_plus(Process* p, Eterm arg1, Eterm arg2);
 Eterm erts_mixed_minus(Process* p, Eterm arg1, Eterm arg2);
 Eterm erts_mixed_times(Process* p, Eterm arg1, Eterm arg2);

erts/emulator/beam/jit/beam_asm.hpp

@@ -1032,6 +1032,8 @@ class BeamGlobalAssembler : public BeamAssembler {
     _(process_main)                                                            \
     _(times_body_shared)                                                       \
     _(times_guard_shared)                                                      \
+    _(unary_minus_body_shared)                                                 \
+    _(unary_minus_guard_shared)                                                \
     _(update_map_assoc_shared)                                                 \
     _(update_map_exact_guard_shared)                                           \
     _(update_map_exact_body_shared)

erts/emulator/beam/jit/instr_arith.cpp

@@ -283,6 +283,82 @@ void BeamModuleAssembler::emit_i_minus(const ArgVal &LHS,
     mov_arg(Dst, RET);
 }
 
+void BeamGlobalAssembler::emit_unary_minus_body_shared() {
+    static const ErtsCodeMFA bif_mfa = {am_erlang, am_Minus, 1};
+
+    Label error = a.newLabel();
+
+    emit_enter_runtime();
+
+    /* Save original arguments for the error path. */
+    a.mov(TMP_MEM1q, ARG2);
+
+    a.mov(ARG1, c_p);
+    runtime_call<2>(erts_unary_minus);
+
+    emit_leave_runtime();
+
+    emit_test_the_non_value(RET);
+    a.short_().je(error);
+
+    a.ret();
+
+    a.bind(error);
+    {
+        /* Place the original argument in x0. */
+        a.mov(ARG1, TMP_MEM1q);
+        a.mov(getXRef(0), ARG1);
+
+        a.mov(ARG4, imm(&bif_mfa));
+        emit_handle_error_shared_prologue();
+    }
+}
+
+void BeamGlobalAssembler::emit_unary_minus_guard_shared() {
+    emit_enter_runtime();
+
+    a.mov(ARG1, c_p);
+    /* ARG2 was set by the caller */
+    runtime_call<2>(erts_unary_minus);
+
+    emit_leave_runtime();
+
+    /* Set ZF if the negation failed. */
+    emit_test_the_non_value(RET);
+    a.ret();
+}
+
+void BeamModuleAssembler::emit_i_unary_minus(const ArgVal &Src,
+                                             const ArgVal &Fail,
+                                             const ArgVal &Dst) {
+    Label next = a.newLabel(), mixed = a.newLabel();
+
+    mov_arg(ARG2, Src);
+    a.mov(RETd, ARG2d);
+    a.and_(RETb, imm(_TAG_IMMED1_MASK));
+    a.cmp(RETb, imm(_TAG_IMMED1_SMALL));
+    a.short_().jne(mixed);
+
+    comment("negation with overflow test");
+    /* RETb is now equal to _TAG_IMMED1_SMALL. */
+    a.movzx(RET, RETb); /* Set RET to make_small(0). */
+    a.mov(ARG3, ARG2);
+    a.and_(ARG3, imm(~_TAG_IMMED1_MASK));
+    a.sub(RET, ARG3);
+    a.short_().jno(next);
+
+    a.bind(mixed);
+    if (Fail.getValue() != 0) {
+        safe_fragment_call(ga->get_unary_minus_guard_shared());
+        a.je(labels[Fail.getValue()]);
+    } else {
+        safe_fragment_call(ga->get_unary_minus_body_shared());
+    }
+
+    a.bind(next);
+    mov_arg(Dst, RET);
+}
+
 /* ARG1 = LHS, ARG4 (!) = RHS
  *
  * We avoid using ARG2 and ARG3 because multiplication clobbers RDX, which is

erts/emulator/beam/jit/ops.tab

@@ -1156,7 +1156,7 @@ gc_bif2 Fail Live u$bif:erlang:splus/2 S1 S2 Dst => \
    gen_plus Fail Live S1 S2 Dst
 
 gc_bif1 Fail Live u$bif:erlang:sminus/1 Src Dst => \
-   gen_minus Fail Live i Src Dst
+   i_unary_minus Src Fail Dst
 gc_bif2 Fail Live u$bif:erlang:sminus/2 S1 S2 Dst => \
    gen_minus Fail Live S1 S2 Dst
 
@@ -1250,6 +1250,8 @@ i_increment S W d
 i_plus s s j? d
 i_minus s s j? d
 
+i_unary_minus s j? d
+
 i_times j? s s d
 
 i_m_div j? s s d

erts/emulator/test/float_SUITE.erl

@@ -24,7 +24,7 @@
 
 -export([all/0, suite/0, groups/0,
          fpe/1,fp_drv/1,fp_drv_thread/1,denormalized/1,match/1,
-         t_mul_add_ops/1,
+         t_mul_add_ops/1,negative_zero/1,
          bad_float_unpack/1, write/1, cmp_zero/1, cmp_integer/1, cmp_bignum/1]).
 -export([otp_7178/1]).
 -export([hidden_inf/1]).
@@ -37,7 +37,7 @@ suite() ->
 all() -> 
     [fpe, fp_drv, fp_drv_thread, otp_7178, denormalized,
      match, bad_float_unpack, write, {group, comparison}
-     ,hidden_inf
+     ,hidden_inf, negative_zero
      ,arith, t_mul_add_ops].
 
 groups() -> 
@@ -56,6 +56,21 @@ otp_7178(Config) when is_list(Config) ->
     {'EXIT', {badarg,_}} = (catch list_to_float("1.0e83291083210")),
     ok.
 
+negative_zero(Config) when is_list(Config) ->
+    <<16#8000000000000000:64>> = do_negative_zero('-', [0.0]),
+    <<16#8000000000000000:64>> = do_negative_zero('*', [-1, 0.0]),
+    <<16#8000000000000000:64>> = do_negative_zero('*', [-1.0, 0.0]),
+    <<16#8000000000000000:64>> = do_negative_zero('*', [-1.0, 0]),
+    ok.
+
+do_negative_zero(Op, Ops) ->
+    Res = <<(my_apply(erlang, Op, Ops))/float>>,
+    Res = <<(case {Op, Ops} of
+                 {'-', [A]} -> -A;
+                 {'*', [A, B]} -> A * B
+             end)/float>>,
+    Res.
+
 %% Forces floating point exceptions and tests that subsequent, legal,
 %% operations are calculated correctly.  Original version by Sebastian
 %% Strollo.