]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
riscv64: Fix nan-boxing for single-precision calculations
authorIvan Tetyushkin <ivan.tetyushkin@syntacore.com>
Mon, 21 Apr 2025 08:59:48 +0000 (11:59 +0300)
committerMark Wielaard <mark@klomp.org>
Fri, 9 May 2025 13:37:59 +0000 (15:37 +0200)
For float values, for arithmetics we expect to have
canonical nan if used double register is not currectly nan-boxed.

https://bugs.kde.org/show_bug.cgi?id=503098

NEWS
VEX/priv/guest_riscv64_toIR.c
none/tests/riscv64/float32.c
none/tests/riscv64/float32.stdout.exp

diff --git a/NEWS b/NEWS
index 1ced90caef2fb8303099a773438fdf929a3d038e..460eb56569b212b70817d41fdd53325610bf640b 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,7 @@ bugzilla (https://bugs.kde.org/enter_bug.cgi?product=valgrind) rather
 than mailing the developers (or mailing lists) directly -- bugs that
 are not entered into bugzilla tend to get forgotten about or ignored.
 
+503098  Incorrect NAN-boxing for float registers in RISC-V
 503641  close_range syscalls started failing with 3.25.0
 503677  duplicated-cond compiler warning in dis_RV64M
 503817  s390x: fix 'ordered comparison of pointer with integer zero' compiler warnings
index 393a7ca7d0546d3802ba0917e9ca964cae9c2470..59297acd15e5ccd4ff69e5416acf37d9e4054456 100644 (file)
@@ -522,9 +522,13 @@ static IRExpr* getFReg32(UInt fregNo)
    vassert(fregNo < 32);
    /* Note that the following access depends on the host being little-endian
       which is checked in disInstr_RISCV64(). */
-   /* TODO Check that the value is correctly NaN-boxed. If not then return
-      the 32-bit canonical qNaN, as mandated by the RISC-V ISA. */
-   return IRExpr_Get(offsetFReg(fregNo), Ity_F32);
+   IRExpr* f64       = getFReg64(fregNo);
+   IRExpr* high_half = unop(Iop_64HIto32, unop(Iop_ReinterpF64asI64, f64));
+   IRExpr* cond      = binop(Iop_CmpEQ32, high_half, mkU32(0xffffffff));
+   IRExpr* res       = IRExpr_ITE(
+      cond, IRExpr_Get(offsetFReg(fregNo), Ity_F32),
+      /* canonical nan */ unop(Iop_ReinterpI32asF32, mkU32(0x7fc00000)));
+   return res;
 }
 
 /* Write a 32-bit value into a guest floating-point register. */
@@ -2160,8 +2164,10 @@ static Bool dis_RV64F(/*MB_OUT*/ DisResult* dres,
       UInt  rs2     = INSN(24, 20);
       UInt  imm11_0 = INSN(31, 25) << 5 | INSN(11, 7);
       ULong simm    = vex_sx_to_64(imm11_0, 12);
-      storeLE(irsb, binop(Iop_Add64, getIReg64(rs1), mkU64(simm)),
-              getFReg32(rs2));
+      // do not modify the bits being transferred;
+      IRExpr* f64 = getFReg64(rs2);
+      IRExpr* i32 = unop(Iop_64to32, unop(Iop_ReinterpF64asI64, f64));
+      storeLE(irsb, binop(Iop_Add64, getIReg64(rs1), mkU64(simm)), i32);
       DIP("fsw %s, %lld(%s)\n", nameFReg(rs2), (Long)simm, nameIReg(rs1));
       return True;
    }
@@ -2456,8 +2462,16 @@ static Bool dis_RV64F(/*MB_OUT*/ DisResult* dres,
        INSN(24, 20) == 0b00000 && INSN(31, 25) == 0b1110000) {
       UInt rd  = INSN(11, 7);
       UInt rs1 = INSN(19, 15);
-      if (rd != 0)
-         putIReg32(irsb, rd, unop(Iop_ReinterpF32asI32, getFReg32(rs1)));
+      if (rd != 0) {
+         // For RV64, the higher 32 bits of the destination register are filled
+         // with copies of the floating-point number’s sign bit.
+         IRExpr* freg      = getFReg64(rs1);
+         IRExpr* low_half  = unop(Iop_64to32, unop(Iop_ReinterpF64asI64, freg));
+         IRExpr* sign      = binop(Iop_And32, low_half, mkU32(1u << 31));
+         IRExpr* cond      = binop(Iop_CmpEQ32, sign, mkU32(1u << 31));
+         IRExpr* high_part = IRExpr_ITE(cond, mkU32(0xffffffff), mkU32(0));
+         putIReg64(irsb, rd, binop(Iop_32HLto64, high_part, low_half));
+      }
       DIP("fmv.x.w %s, %s\n", nameIReg(rd), nameFReg(rs1));
       return True;
    }
index b63305a64eac3639d1c5576647aea19fcf1f85f6..f635bc76147884d6a1fb86391682d6144d4872d5 100644 (file)
@@ -1578,6 +1578,12 @@ static void test_float32_additions(void)
    TESTINST_1_1_FI(4, "fcvt.s.lu fa0, a0", 0x0000000001000001, 0x60, fa0, a0);
    /* 2**24+1 (DYN-RMM) -> 2**24+2 (NX) */
    TESTINST_1_1_FI(4, "fcvt.s.lu fa0, a0", 0x0000000001000001, 0x80, fa0, a0);
+
+   // check nan-boxing
+   /* fabs.s rd, rs1 */
+   TESTINST_1_2_F(4, "fsgnjx.s fa0, fa1, fa1", 0xfaffffff3f800000,
+                  0xfaffffff3f800000, 0x00, fa0, fa1, fa1);
+
 }
 
 int main(void)
index 013c7eda21f1d67eadcef8fa66600af1c3c58278..734370d518bc20dc1dc9768000238be72ae2a475 100644 (file)
@@ -1554,3 +1554,6 @@ fcvt.s.lu fa0, a0 ::
 fcvt.s.lu fa0, a0 ::
   inputs: a0=0x0000000001000001, fcsr=0x00000080
   output: fa0=0xffffffff4b800001, fcsr=0x00000081
+fsgnjx.s fa0, fa1, fa1 ::
+  inputs: fa1=0xfaffffff3f800000, fa1=0xfaffffff3f800000, fcsr=0x00000000
+  output: fa0=0xffffffff7fc00000, fcsr=0x00000000