[RISCV][FPEnv] Lowering of fpenv intrinsics

[spavloff]

The change implements custom lowering of get_fpenv, set_fpenv and reset_fpenv for RISCV target.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Serge Pavlov (spavloff)

Changes

The change implements custom lowering of get_fpenv, set_fpenv and reset_fpenv for RISCV target.

---

Full diff: https://github.com/llvm/llvm-project/pull/141498.diff

4 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+3)
• (modified) llvm/lib/Target/RISCV/RISCVInstrInfoF.td (+11)
• (added) llvm/test/CodeGen/RISCV/fpenv32.ll (+33)
• (added) llvm/test/CodeGen/RISCV/fpenv64.ll (+33)

diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 0a849f49116ee..b4f8038041e78 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -649,6 +649,9 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
 
     setOperationAction(ISD::GET_ROUNDING, XLenVT, Custom);
     setOperationAction(ISD::SET_ROUNDING, MVT::Other, Custom);
+    setOperationAction(ISD::GET_FPENV, XLenVT, Legal);
+    setOperationAction(ISD::SET_FPENV, XLenVT, Legal);
+    setOperationAction(ISD::RESET_FPENV, MVT::Other, Legal);
   }
 
   setOperationAction({ISD::GlobalAddress, ISD::BlockAddress, ISD::ConstantPool,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoF.td b/llvm/lib/Target/RISCV/RISCVInstrInfoF.td
index 84a75666e5f36..9edfe34e0b67b 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoF.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoF.td
@@ -736,6 +736,17 @@ def : LdPat<load, LW_INX, f32>;
 def : StPat<store, SW_INX, GPRF32, f32>;
 } // Predicates = [HasStdExtZfinx]
 
+/// Floating-point environment
+multiclass FPEnvironmentOps<Predicate HasFloatExt> {
+  let Predicates = [HasFloatExt] in {
+    def : Pat<(XLenVT (get_fpenv)), (CSRRS SysRegFCSR.Encoding, (XLenVT X0))>;
+    def : Pat<(set_fpenv (XLenVT GPR:$rs)), (CSRRW SysRegFCSR.Encoding, GPR:$rs)>;
+    def : Pat<(reset_fpenv), (CSRRW SysRegFCSR.Encoding, (XLenVT X0))>;
+  }
+}
+defm : FPEnvironmentOps<HasStdExtF>;
+defm : FPEnvironmentOps<HasStdExtZfinx>;
+
 let Predicates = [HasStdExtF, IsRV32] in {
 // Moves (no conversion)
 def : Pat<(bitconvert (i32 GPR:$rs1)), (FMV_W_X GPR:$rs1)>;
diff --git a/llvm/test/CodeGen/RISCV/fpenv32.ll b/llvm/test/CodeGen/RISCV/fpenv32.ll
new file mode 100644
index 0000000000000..4d4976d391f80
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/fpenv32.ll
@@ -0,0 +1,33 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=riscv32 -mattr=+f -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv32 -mattr=+zfinx -verify-machineinstrs < %s | FileCheck %s
+
+define i32 @func_get_fpenv() {
+; CHECK-LABEL: func_get_fpenv:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    frcsr a0
+; CHECK-NEXT:    ret
+entry:
+  %fpenv = call i32 @llvm.get.fpenv.i32()
+  ret i32 %fpenv
+}
+
+define void @func_set_fpenv(i32 %fpenv) {
+; CHECK-LABEL: func_set_fpenv:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    fscsr a0
+; CHECK-NEXT:    ret
+entry:
+  call void @llvm.set.fpenv.i32(i32 %fpenv)
+  ret void
+}
+
+define void @func_reset_fpenv() {
+; CHECK-LABEL: func_reset_fpenv:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    fscsr zero
+; CHECK-NEXT:    ret
+entry:
+  call void @llvm.reset.fpenv()
+  ret void
+}
diff --git a/llvm/test/CodeGen/RISCV/fpenv64.ll b/llvm/test/CodeGen/RISCV/fpenv64.ll
new file mode 100644
index 0000000000000..3a7a455c3eed1
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/fpenv64.ll
@@ -0,0 +1,33 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=riscv64 -mattr=+f -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+zfinx -verify-machineinstrs < %s | FileCheck %s
+
+define i64 @func_get_fpenv() {
+; CHECK-LABEL: func_get_fpenv:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    frcsr a0
+; CHECK-NEXT:    ret
+entry:
+  %fpenv = call i64 @llvm.get.fpenv.i64()
+  ret i64 %fpenv
+}
+
+define void @func_set_fpenv(i64 %fpenv) {
+; CHECK-LABEL: func_set_fpenv:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    fscsr a0
+; CHECK-NEXT:    ret
+entry:
+  call void @llvm.set.fpenv.i64(i64 %fpenv)
+  ret void
+}
+
+define void @func_reset_fpenv() {
+; CHECK-LABEL: func_reset_fpenv:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    fscsr zero
+; CHECK-NEXT:    ret
+entry:
+  call void @llvm.reset.fpenv()
+  ret void
+}

[topperc]

I'm trying to decide how concerned we should be that this generates fscsr a0, a0 with -O0. The replacement of the destination with x0 to discard it is treated as an optimization.

[spavloff]

The choice here is determined by compatibility with GLIBC, which uses this form:

https://github.com/bminor/glibc/blob/08d7243a6179d5a1f3f65a53aba1ec0803895aeb/sysdeps/riscv/fpu_control.h#L44

[topperc]

gcc always uses fscsr a0. With the current patch, -O0 will use fscsr a0, a0 and -O1 and above will use fscsr a0. The one operand form implicitly uses X0 as the destination which is a special encoding to hardware that indicates that no read needs to be performed. The X0 destination is currently being set by RISCVDeadRegisterDefinitions.cpp.

To fix -O0 to always use X0 destination, we need a pseudo instruction that doesn't have a destination that we will expand to to use X0 during RISCVAsmPrinter.

[spavloff]

Thank you for explanation. The new inplementation uses custom lowering, which is also necessary to maintain correct implicit reg dependencies.

[topperc]

Do we need hasSideEffects like the ReadFFLAGS and WriteFFLAGS above?

[spavloff]

It looks like too rigid restriction. ReadFFLAGS and WriteFFLAGS already have FRM and FFLAGS as inplicit use/def, shouldn't that be sufficient to provide the necessary ordering?