Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
compiler-rt/lib/builtins/riscv/fp_mode.c | ||
---|---|---|
40–41 | Can't this be replaced with a single csrrs? |
compiler-rt/lib/builtins/riscv/fp_mode.c | ||
---|---|---|
40–41 | More specifically, csrrsi, as RISCV_INEXACT fits the 5-bit immediate. |
compiler-rt/lib/builtins/riscv/fp_mode.c | ||
---|---|---|
40–41 | 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? |
compiler-rt/lib/builtins/riscv/fp_mode.c | ||
---|---|---|
40–41 | 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. |
compiler-rt/lib/builtins/riscv/fp_mode.c | ||
---|---|---|
19 | D implies F, it's redundant | |
40 |
|
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." |
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. |
- Delete checking if __riscv_d was defined since d extension imply f extension
- Replace csrrsi to csrsi
- Use fflags instead of its csr number to make code readable
compiler-rt/lib/builtins/riscv/fp_mode.c | ||
---|---|---|
40 | Thanks for your detailed explanation! |
compiler-rt/lib/builtins/riscv/fp_mode.c | ||
---|---|---|
40 | 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. |
LGTM.
compiler-rt/lib/builtins/riscv/fp_mode.c | ||
---|---|---|
40 | Nevermind. I checked the spec, NX is indeed there in fflags. |
D implies F, it's redundant