[RISCV] Support fe_getround and fe_raise_inexact in builtins

[StephenFan]

update.

[luismarques]

compiler-rt/lib/builtins/riscv/fp_mode.c

41–42

Can't this be replaced with a single csrrs?

[luismarques]

compiler-rt/lib/builtins/riscv/fp_mode.c

41–42

More specifically, csrrsi, as RISCV_INEXACT fits the 5-bit immediate.

[StephenFan]

compiler-rt/lib/builtins/riscv/fp_mode.c

41–42

I am a beginner at inline assembly programming. The statement flags | RISCV_INEXACT's value is only known at runtime instead of compile-time, can we still use csrrsi instruction to set the float flags?

[jrtc27]

compiler-rt/lib/builtins/riscv/fp_mode.c

41–42

That's the point of csrrsi, the S stands for Set, i.e. it will OR the value you provide with the current one for you, no need to read the existing value and do the OR yourself.

[StephenFan]

Replace fsflags with csrrsi.

[jrtc27]

compiler-rt/lib/builtins/riscv/fp_mode.c

19

D implies F, it's redundant

40

1. You want csrrsi x0 (alias csrsi as the nice way to write it), not t0, otherwise you're clobbering a random register the compiler could be using (and not telling it, but in this case you don't need to clobber it in the first place).
2. Why 0x001 rather than the readable fflags?

[StephenFan]

compiler-rt/lib/builtins/riscv/fp_mode.c

40

The spec says: "Further assembler pseudoinstructions are defined to set and clear bits in the CSR when the old value is not required: CSRS/CSRC csr, rs1; CSRSI/CSRCI csr, uimm."
So the csrsi fflags, 0x1 sets the fflags to 0x1 directly instead of 0x1 | fflags.Is my understanding correct?

[jrtc27]

compiler-rt/lib/builtins/riscv/fp_mode.c

40

No, "the old value is not required" means that you don't want to write the old value to a register for use outside the instruction. Your "set fflags to 0x1 directly" would be csrwi fflags, 0x1 instead. csrsi fflags, 0x1 does exactly what you need here, and is merely another name for csrrsi x0, fflags.

[StephenFan]

1. Delete checking if __riscv_d was defined since d extension imply f extension
2. Replace csrrsi to csrsi
3. Use fflags instead of its csr number to make code readable

[StephenFan]

compiler-rt/lib/builtins/riscv/fp_mode.c

40

Thanks for your detailed explanation!

[StephenFan]

Ping.

[StephenFan]

Ping.

[luismarques]

compiler-rt/lib/builtins/riscv/fp_mode.c

39

Is this actually supposed to be fflags or frm? The frm field inside fcsr starts at bit 5 so you can't just do csrsi fcsr, 1 (RISCV_INEXACT == 0x1). It would need to be csrsi fcsr, 1 << 5 (but that exceeds the 5-bit immediate, so it's not actually possible with the immediate instruction variant). I think the frm field of fcsr is also mirrored as a separate CSR, frm, so you can still use csrsi through there.

[luismarques]

LGTM.

compiler-rt/lib/builtins/riscv/fp_mode.c

39

Nevermind. I checked the spec, NX is indeed there in fflags.

compiler-rt/lib/builtins/CMakeLists.txt

set(powerpc64_SOURCES
     ppc/fixtfti.c
     ppc/fixunstfti.c
     ${powerpc64_SOURCES}
   )
 endif()
 set(powerpc64le_SOURCES ${powerpc64_SOURCES})
 
 set(riscv_SOURCES
+  riscv/fp_mode.c
   riscv/save.S
   riscv/restore.S
   ${GENERIC_SOURCES}
   ${GENERIC_TF_SOURCES}
 )
 set(riscv32_SOURCES
   riscv/mulsi3.S
   ${riscv_SOURCES}

compiler-rt/lib/builtins/riscv/fp_mode.c

+//=== lib/builtins/riscv/fp_mode.c - Floaing-point mode utilities -*- C -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#include "../fp_mode.h"
+
+#define RISCV_TONEAREST  0x0
+#define RISCV_TOWARDZERO 0x1
+#define RISCV_DOWNWARD   0x2
+#define RISCV_UPWARD     0x3
+
+#define RISCV_INEXACT    0x1
+
+CRT_FE_ROUND_MODE __fe_getround(void) {
+#if defined(__riscv_f)
+  int frm;
+  __asm__ __volatile__("frrm %0" : "=r" (frm));
+  switch (frm) {
+    case RISCV_TOWARDZERO:
+      return CRT_FE_TOWARDZERO;
+    case RISCV_DOWNWARD:
+      return CRT_FE_DOWNWARD;
+    case RISCV_UPWARD:
+      return CRT_FE_UPWARD;
+    case RISCV_TONEAREST:
+    default:
+      return CRT_FE_TONEAREST;
+  }
+#else
+  return CRT_FE_TONEAREST;
+#endif
+}
+
+int __fe_raise_inexact(void) {
+#if defined(__riscv_f)
+  __asm__ __volatile__("csrsi fflags, %0" :: "i" (RISCV_INEXACT));
+#endif
+  return 0;
+}