[llvm] Incorrect simplification of fabs intrinsic

[ojhunt]

The below test case fails at optimization levels greater than -O1

#include <math.h>
#include <stdio.h>

__attribute__((noinline))
int test(double a, double b)
{
  int maxIdx = 0;
  double maxDot = 0.0;
  for (int i = 0; i < 2; ++i) {
    double dot = a * b;
    if (fabs(dot) > fabs(maxDot)) {
      maxIdx = i;
      maxDot = dot;
    }
  }
  return maxIdx;
}

int main(int argc, char *argv[])
{

  int i = test(1.0, -1.0);
  int failed = (i != 0);

  printf("Returned %d, should be 0: %s\n", i, (failed ? "failed" : "success"));

  return failed;
}

This is a regression introduced in #76360

The bug is due to https://github.com/llvm/llvm-project/pull/76360/files#diff-532fd830a7b080a7121645e6c8dccb2f94a7e25450c74aa496b8600f8f4481ccR6220

comparing a std::optional to false rather than dereferencing it (maybe we need a way to warn about this? it's a super easy error)

[Implicit conversion means the comparison I thought was wrong, was correct, and my PoC fix worked by functionally disabling signedness propagation entirely]

[nikic]

Seems to go wrong during unroling: https://alive2.llvm.org/ce/z/sxJ5eo

[nikic]

Or rather in simplification during unrolling (https://alive2.llvm.org/ce/z/66EoES):

define half @src(half noundef %mul) {
entry:
  %abs = call half @llvm.fabs.f16(half %mul)
  %cmp1 = fcmp ogt half %abs, 0.000000e+00
  %maxDot.1 = select i1 %cmp1, half %mul, half 0.000000e+00
  %abs2 = call half @llvm.fabs.f16(half %maxDot.1)
  ret half %abs2
}

define half @tgt(half noundef %mul) {
entry:
  %abs = call half @llvm.fabs.f16(half %mul)
  %cmp1 = fcmp ogt half %abs, 0xH0000
  %maxDot.1 = select i1 %cmp1, half %mul, half 0xH0000
  ret half %maxDot.1
}

The second abs should not be eliminated.

[nikic]

Presumably something is wrong in

llvm-project/llvm/lib/Analysis/ValueTracking.cpp

Lines 4852 to 4860 in fc44a4f

	if (F && match(Cond, m_FCmp(Pred, m_Value(CmpLHS), m_Value(CmpRHS)))) {
	// If the select filters out a value based on the class, it no longer
	// participates in the class of the result
	// TODO: In some degenerate cases we can infer something if we try again
	// without looking through sign operations.
	bool LookThroughFAbsFNeg = CmpLHS != LHS && CmpLHS != RHS;
	std::tie(TestedValue, MaskIfTrue, MaskIfFalse) =
	fcmpImpliesClass(Pred, *F, CmpLHS, CmpRHS, LookThroughFAbsFNeg);

or the fcmpImpliesClass utility it calls.

[dtcxzyw]

@nikic @ojhunt Are you working on this issue?

[nikic]

I'm not working on it.