Incorrect code generated for float llvm.maximum/minimum intrinsics

[danilaml]

I believe this change introduced a bug preventing correct handing of signed zeroes in certain cases.

This check

  if (Op->getOpcode() == ISD::BUILD_VECTOR ||
      Op->getOpcode() == ISD::SPLAT_VECTOR) {
    for (const SDValue &OpVal : Op->op_values()) {
      if (OpVal.isUndef())
        return false;
      if (auto *C = dyn_cast<ConstantFPSDNode>(OpVal))
        if (C->isZero())
          return false;
    }
    return true;
  }

Returns true by default, marking most BUILD/SPLAT vectors as known non-zeroes preventing the negative zero check generation.

See this example: https://godbolt.org/z/Yh5oE916v

target triple = "x86_64-unknown-linux-gnu"

declare float @llvm.maximum.f32(float, float)
declare <8 x float> @llvm.maximum.v8f32(<8 x float>,<8 x float>)

define <8 x float> @foo(float %a, <8 x float> %vec) #0 {
entry:
  %splatinsert = insertelement <8 x float> poison, float %a, i64 0
  %splat = shufflevector <8 x float> %splatinsert, <8 x float> poison, <8 x i32> zeroinitializer
  %res = call nnan noundef <8 x float> @llvm.maximum.v8f32(<8 x float> %splat, <8 x float> %vec)
  ret <8 x float> %res
}

attributes #0 = { "target-cpu"="haswell" }

I believe the check should look something like

  if (Op->getOpcode() == ISD::BUILD_VECTOR ||
      Op->getOpcode() == ISD::SPLAT_VECTOR) {
    return llvm::all_of(Op->op_values(), [](const SDValue &OpVal){
      if (auto *C = dyn_cast<ConstantFPSDNode>(OpVal))
        return !C->isZero();
      return false;
    });
  }

[llvmbot]

@llvm/issue-subscribers-backend-x86

Author: Danila Malyutin (danilaml)

I believe [this change](https://github.com/llvm/llvm-project/commit/7bff37783f72ed99e4cdd0fd7dad1bf1f119f793) introduced a bug preventing correct handing of signed zeroes in certain cases.

This check

  if (Op->getOpcode() == ISD::BUILD_VECTOR ||
      Op->getOpcode() == ISD::SPLAT_VECTOR) {
    for (const SDValue &OpVal : Op->op_values()) {
      if (OpVal.isUndef())
        return false;
      if (auto *C = dyn_cast<ConstantFPSDNode>(OpVal))
        if (C->isZero())
          return false;
    }
    return true;
  }

Returns true by default, marking most BUILD/SPLAT vectors as known non-zeroes preventing the negative zero check generation.

See this example: https://godbolt.org/z/Yh5oE916v

target triple = "x86_64-unknown-linux-gnu"

declare float @<!-- -->llvm.maximum.f32(float, float)
declare &lt;8 x float&gt; @<!-- -->llvm.maximum.v8f32(&lt;8 x float&gt;,&lt;8 x float&gt;)

define &lt;8 x float&gt; @<!-- -->foo(float %a, &lt;8 x float&gt; %vec) #<!-- -->0 {
entry:
  %splatinsert = insertelement &lt;8 x float&gt; poison, float %a, i64 0
  %splat = shufflevector &lt;8 x float&gt; %splatinsert, &lt;8 x float&gt; poison, &lt;8 x i32&gt; zeroinitializer
  %res = call nnan noundef &lt;8 x float&gt; @<!-- -->llvm.maximum.v8f32(&lt;8 x float&gt; %splat, &lt;8 x float&gt; %vec)
  ret &lt;8 x float&gt; %res
}

attributes #<!-- -->0 = { "target-cpu"="haswell" }

I believe the check should look something like

  if (Op->getOpcode() == ISD::BUILD_VECTOR ||
      Op->getOpcode() == ISD::SPLAT_VECTOR) {
    return llvm::all_of(Op->op_values(), [](const SDValue &OpVal){
      if (auto *C = dyn_cast<ConstantFPSDNode>(OpVal))
        return C->isZero();
      return false;
    });
  }

[RKSimon]

CC @sparker-arm

[sparker-arm]

Sorry for breaking things... but I'm not going to have time to fix this right now.

[RKSimon]

I'll see if I can fix this

[danilaml]

@RKSimon I can prepare my suggestion above for a review, if you want.

EDIT: went ahead and submitted it, since I had everything mostly ready.