SDAG doesn't handle signed zero correctly when folding select -> {min|max}num

dtcxzyw commented 3 months ago

Reproducer: https://godbolt.org/z/sqYqK8vd9

; llc -mtriple=riscv64 -mattr=+f test.ll -o -
define float @test(float %1) {
entry:
  %2 = fcmp olt float %1, 0.000000
  %3 = select i1 %2, float %1, float 0.000000
  ret float %3
}

test:                                   # @test
        fmv.w.x fa5, zero
        fmin.s  fa0, fa0, fa5
        ret

When %1 is -0.0, the former returns +0.0 while the latter returns -0.0. SelectionDAGBuilder::visitSelect incorrectly refined the select (always returns +0.0 when the input is -0.0) into ISD::FMINNUM (returns either 0.0 or -0.0). Related code: https://github.com/llvm/llvm-project/blob/a4a436672a2c179274e07aeb68e9acd6f483a653/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp#L3718-L3754

cc @topperc @arsenm

dtcxzyw commented 3 months ago

Original test case: https://github.com/dtcxzyw/llvm-codegen-benchmark/blob/main/dataset/75a2ca2dbd7ae03a.ll

arsenm commented 3 months ago

We really need a version of ISD::FMINNUM/FMAXNUM that always respect the 0 sign. This would also be a lot easier if we just fix the definition of minnum/maxnum to have the IEEE snan behavior.

Also, can we just get rid of this IR pattern matching in SelectionDAGBuilder? We already have to repeat the same matching on the DAG node, and it's really difficult to test when the DAG builder has hidden optimizations in it

llvm / llvm-project

SDAG doesn't handle signed zero correctly when folding select -> {min|max}num #93414