Page MenuHomePhabricator

[RISCV] Custom lowering of SET_ROUNDING
Needs ReviewPublic

Authored by sepavloff on Nov 11 2020, 2:37 AM.

Diff Detail

Unit TestsFailed

TimeTest
370 mslinux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -mcmodel=large -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/hwasan/TestCases/sizes.cpp -nostdlib++ -lstdc++ -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/sizes.cpp.tmp

Event Timeline

sepavloff created this revision.Nov 11 2020, 2:37 AM
sepavloff requested review of this revision.Nov 11 2020, 2:37 AM

Would a SELECT not be more friendly to optimisations like known-bits analysis, or does that cause too much code to be emitted in the general case?

craig.topper added inline comments.Nov 11 2020, 9:37 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
225

StdExtD implies StdExtF. So I think you only need to check StdExtF

904

Why do we need two code paths? Won't the other code constant fold if RMValue is a constant?

sepavloff updated this revision to Diff 304799.Nov 12 2020, 5:45 AM

Updated patch according to reviewer's notes.

Would a SELECT not be more friendly to optimisations like known-bits analysis, or does that cause too much code to be emitted in the general case?

Not sure if I understand you correctly, if you mean analyzing the 3 bits of rounding mode bit-by-bit, It think it does not make sense here. It is unlikely that only a part of the bits are known. Rounding mode is specified as a constant or obtained as a result of FLT_ROUNDS or as argument of a function call. Usually it is not obtained by calculations. And yes, in the general case too much code is generated, at least 8 operators, while in this implementation 5 instructions is enough.

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

Fixed.

904

Indeed, the general case can be optimized for constant arguments.

If this gets used in a function that does floating point, what prevents the machine schedulers from reordering the FP instructions with this? Don't we need to model FRM as a register and make the FP instructions implicitly use it? You'd also need to be using the constrained FP intrinsics which aren't implemented for RISCV yet.

lenary resigned from this revision.Jan 14 2021, 9:44 AM