Page MenuHomePhabricator

[RISCV] Introduce floating point control and state registers
Needs ReviewPublic

Authored by sepavloff on Mon, Mar 22, 8:57 AM.

Details

Summary

New registers FRM, FFLAGS and FCSR was defined. They represent
corresponding system registers. The new registers are necessary to
properly order floating point instructions in non-default modes.

Diff Detail

Unit TestsFailed

TimeTest
2,140 msx64 debian > libarcher.races::lock-unrelated.c
Script: -- : 'RUN: at line 13'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests -I /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/llvm-project/build/lib -Wl,-rpath,/mnt/disks/ssd0/agent/llvm-project/build/lib /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/races/lock-unrelated.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/deflake.bash /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp 2>&1 | tee /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp.log | /mnt/disks/ssd0/agent/llvm-project/build/./bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/races/lock-unrelated.c

Event Timeline

sepavloff created this revision.Mon, Mar 22, 8:57 AM
sepavloff requested review of this revision.Mon, Mar 22, 8:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Mar 22, 8:57 AM
Herald added a subscriber: MaskRay. · View Herald Transcript
craig.topper added inline comments.Wed, Mar 31, 10:57 AM
llvm/lib/Target/RISCV/RISCVInstrInfo.td
1164

Should we use the names of the mnemonics that are defined for these operations? Though I guess there's no name defined for "write" just for swap and read.

sepavloff added inline comments.Wed, Mar 31, 11:50 PM
llvm/lib/Target/RISCV/RISCVInstrInfo.td
1164

Did you mean frrm and fsrm? Name like Read* are more readable. Besides it seems there are no advantages of having codegen pseudos named identically to corresponding instruction aliases. And you are right, Write* and Swap* correspond to the same mnemonic.

sepavloff updated this revision to Diff 334904.Thu, Apr 1, 11:43 PM

Rebased ad added variants with immediate

craig.topper added inline comments.Wed, Apr 7, 11:34 AM
llvm/lib/Target/RISCV/RISCVInstrInfo.td
1173

Do we have use cases for all of these?

llvm/lib/Target/RISCV/RISCVRegisterInfo.td
543

Is there any significance the 1, 2, and 3 chosen for the first operand here?

sepavloff added inline comments.Wed, Apr 7, 8:47 PM
llvm/lib/Target/RISCV/RISCVInstrInfo.td
1173

ReadFRM is used in the implementation of FLT_ROUNDS_ (D90854).
WriteFRM and WrireFRMImm are used in the implementation of set_rounding (D91242).
SwapFRM and SwapFRMImm are not used in any of pending patches but they are useful to implement target-specific optimization of the code pattern:

int old_rm = fegetround();
fesetround(new_rm);
...
fesetround(old_rm);

which is likely to be used in the implementation of #pragma STDC FENV_ROUND.

Operations with FFLAGS are not used now. They would be needed to implement functions like fetestexcept and similar, but now there are no such attempts AFAIK.

Operations would be needed for target specific implementation of intrinsics that access FP control modes (D82525) and FP environment (D71742).

llvm/lib/Target/RISCV/RISCVRegisterInfo.td
543

The first operand of RISCVReg is HWEncoding, 1, 2 and 3 represent hardware addresses of the respective system registers which are specified in the RISCV specification (https://github.com/riscv/riscv-isa-manual/releases/download/draft-20210402-1271737/riscv-spec.pdf), chapter 25 (RV32/64G Instruction Set Listings), table 25.3 (RISC-V control and status register (CSR) address map).

On the other hand, these special registers are not used as operands in any of non-pseudo instruction, so these definitions are not used in instruction emitter. It means that particular values are not important, at least now.

sepavloff added inline comments.Wed, Apr 7, 8:48 PM
llvm/lib/Target/RISCV/RISCVInstrInfo.td
1173

Operations would be needed for target specific implementation of intrinsics that access FP control modes (D82525) and FP environment (D71742).

It is about the operation on FCSR.

craig.topper added inline comments.Wed, Apr 7, 9:46 PM
llvm/lib/Target/RISCV/RISCVInstrInfo.td
1173

I don't think we want unused pseudos that might be used someday. We should include them when they're needed.

llvm/lib/Target/RISCV/RISCVRegisterInfo.td
543

I think these should probably just be 0 like the flags registers on other targets. It's only a 5 bit field and the CSR addresses are 12 bits so its purely a coincidence that these CSRs happen to be representable in 5 bits.

sepavloff updated this revision to Diff 336010.Wed, Apr 7, 11:18 PM

Use zero for encoding of FFLAGS, FRM and FCSR. Rebased.