Page MenuHomePhabricator

Correct instcombine of fcmp+select.
Needs ReviewPublic

Authored by csigg on Jan 25 2022, 4:46 AM.

Details

Reviewers
spatel
Summary
  1. Zero fcmp arguments were previously replaced with zero select arguments without considering or adjusting the predicate. However, without no-signed-zeros, only the following are equivalent:
   >= +0  <=>     > -0
   <= -0  <=>     < +0
-0 >=     <=>  +0 >
+0 <=     <=>  -0 <
  1. Combining fcmp+select into fminnum is not permitted when the arguments might be two zeros of opposite sign. E.g., fminnum(-0, +0) can return any of the two operands and is therefore not equivalent to '-0 < +0 ? -0 : +0'. This was handled for >= and <= but marked as 'FIXME' for > and <.
  1. fminimum does treat -0 as smaller than +0, so fcmp+select can be combined if the NaN-propagating bahaviour of fminimum is permitted. Previously, the handling of signed zeros applied to combining fcmp+select into both fminnum and fminimum.

The same applies for fmaxnum/fmaximum.

Hopefully 3) can make up for the performance lost by fixing 1) and 2).

Diff Detail

Unit TestsFailed

TimeTest
60,100 msx64 debian > AddressSanitizer-x86_64-linux-dynamic.TestCases::scariness_score_test.cpp
Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m64 -shared-libasan -O0 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/asan/TestCases/scariness_score_test.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/asan/X86_64LinuxDynamicConfig/TestCases/Output/scariness_score_test.cpp.tmp
60,150 msx64 debian > AddressSanitizer-x86_64-linux.TestCases::scariness_score_test.cpp
Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m64 -O0 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/asan/TestCases/scariness_score_test.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/asan/X86_64LinuxConfig/TestCases/Output/scariness_score_test.cpp.tmp
60,140 msx64 debian > Clang.CodeGen/RISCV/rvv-intrinsics::vloxseg.c
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/14.0.0/include -nostdsysteminc -triple riscv64 -target-feature +f -target-feature +d -target-feature +zfh -target-feature +v -disable-O0-optnone -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vloxseg.c -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -S -mem2reg | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-RV64 /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vloxseg.c
60,140 msx64 debian > Clang.CodeGen/RISCV/rvv-intrinsics::vluxseg.c
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/14.0.0/include -nostdsysteminc -triple riscv64 -target-feature +f -target-feature +d -target-feature +zfh -target-feature +v -disable-O0-optnone -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vluxseg.c -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -S -mem2reg | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-RV64 /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vluxseg.c
90 msx64 debian > LLVM.CodeGen/SystemZ::vec-max-05.ll
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/SystemZ/vec-max-05.ll -mtriple=s390x-linux-gnu -mcpu=z14 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/SystemZ/vec-max-05.ll
View Full Test Results (8 Failed)

Event Timeline

csigg created this revision.Jan 25 2022, 4:46 AM
csigg requested review of this revision.Jan 25 2022, 4:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 25 2022, 4:46 AM
csigg updated this revision to Diff 403571.Jan 27 2022, 3:14 AM

Rebase.

csigg updated this revision to Diff 459756.Sep 13 2022, 8:19 AM

Rebase.

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 13 2022, 8:19 AM

Something has gone wrong with the diff.

Sorry - I didn't remember this patch when I was looking at a bug fix with:
4e44c22c97be

We may need more unit and/or IR tests to verify that we are getting the correct results for all of these patterns.

Something has gone wrong with the diff.

Very much so, I rebased the wrong diff :-(

Let me try to resurrect it.

csigg updated this revision to Diff 459819.Sep 13 2022, 11:11 AM

Restore diff.

Restore diff.

Still need to update to account for the changes 4e44c22c97be?