Page MenuHomePhabricator

[RISCV][RVV] Add FPRndModeOp to PseudoVFCVT instructions
Needs ReviewPublic

Authored by arcbbb on Mar 31 2022, 8:20 PM.

Details

Summary

Rounding mode rtz & rod are supported by vfcvt statically,
while other rounding modes such as rdn & rup are supported dynamically and controlled by FRM register, which also affects scalar floating-point operations.

This patch adds an additional operand to carry round-mode information in vfcvt pseudos.
Lowering and instruction selection can specify the needed round mode imm. value and let the backend deal with the round mode change at MachineInstr level at later stage.

Diff Detail

Unit TestsFailed

TimeTest
60,180 msx64 debian > AddressSanitizer-Unit._/Asan-x86_64-calls-Dynamic-Test/21::49
Script(shard): -- GTEST_OUTPUT=json:/var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/lib/asan/tests/X86_64LinuxDynamicConfig/./Asan-x86_64-calls-Dynamic-Test-AddressSanitizer-Unit-2527885-21-49.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=49 GTEST_SHARD_INDEX=21 /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/lib/asan/tests/X86_64LinuxDynamicConfig/./Asan-x86_64-calls-Dynamic-Test
60,190 msx64 debian > AddressSanitizer-Unit._/Asan-x86_64-calls-Dynamic-Test/33::49
Script(shard): -- GTEST_OUTPUT=json:/var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/lib/asan/tests/X86_64LinuxDynamicConfig/./Asan-x86_64-calls-Dynamic-Test-AddressSanitizer-Unit-2527885-33-49.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=49 GTEST_SHARD_INDEX=33 /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/lib/asan/tests/X86_64LinuxDynamicConfig/./Asan-x86_64-calls-Dynamic-Test
60,210 msx64 debian > AddressSanitizer-Unit._/Asan-x86_64-calls-Noinst-Test/12::16
Script(shard): -- GTEST_OUTPUT=json:/var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/lib/asan/tests/X86_64LinuxConfig/./Asan-x86_64-calls-Noinst-Test-AddressSanitizer-Unit-2527885-12-16.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=16 GTEST_SHARD_INDEX=12 /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/lib/asan/tests/X86_64LinuxConfig/./Asan-x86_64-calls-Noinst-Test
60,380 msx64 debian > AddressSanitizer-Unit._/Asan-x86_64-calls-Noinst-Test/9::16
Script(shard): -- GTEST_OUTPUT=json:/var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/lib/asan/tests/X86_64LinuxConfig/./Asan-x86_64-calls-Noinst-Test-AddressSanitizer-Unit-2527885-9-16.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=16 GTEST_SHARD_INDEX=9 /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/lib/asan/tests/X86_64LinuxConfig/./Asan-x86_64-calls-Noinst-Test
60,040 msx64 debian > libFuzzer.libFuzzer::large.test
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/fuzzer -m64 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/fuzzer/LargeTest.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/large.test.tmp-LargeTest

Event Timeline

arcbbb created this revision.Mar 31 2022, 8:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 8:20 PM
arcbbb requested review of this revision.Mar 31 2022, 8:20 PM

This patch only creates the interface to specify the round mode in vfcvt pseudos.
How the round mode is changed at MI will be a separate patch.
I think this can make each patch smaller.

Thanks @arcbbb.

Just to understand the design principle behind these patches: adding FRM as Use to the v*cvt instructions (like we did in D121087 ) would not help us to implement floor and ceil. I imagine one option could be adding specific pseudos for round up and round down and then a later pass could set the rounding mode and restore it later. However, this would not give great code generation and it adds even more instructions to our already large number of pseudos. So your approach goes by adding a new operand with the rounding mode, similar to the scalar operations, and in D123371 you propose a pass that adjusts the FRM register.

I hope I got it right :)

llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
99

Could you add a comment explaining what this flag means?

What I understand is that for "vfcvt" instructions we annotate the rounding mode. It'll be commonly dynamic except for cases when we want to explicitly round up or down.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9532

It looks like you could factor this with the code below, it is very similar.

arcbbb updated this revision to Diff 426299.May 1 2022, 9:28 AM

Address @rogfer01's comments

arcbbb marked 2 inline comments as done.May 1 2022, 9:31 AM

Thanks @arcbbb.

Just to understand the design principle behind these patches: adding FRM as Use to the v*cvt instructions (like we did in D121087 ) would not help us to implement floor and ceil. I imagine one option could be adding specific pseudos for round up and round down and then a later pass could set the rounding mode and restore it later. However, this would not give great code generation and it adds even more instructions to our already large number of pseudos. So your approach goes by adding a new operand with the rounding mode, similar to the scalar operations, and in D123371 you propose a pass that adjusts the FRM register.

I hope I got it right :)

Many thanks for your summary. It states much better than mine.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
99

Thanks for the reminding!