[llvm] [X86] Fix miscompile of fptosi.sat.iN.f16 for NaN under avx512fp16 (PR #210556)

[Nathaniel McCallum via llvm-commits]

npmccallum wrote:

Thanks — restructured along the lines you suggested.

The standalone `avx512fp16-fptosi-sat-nan.ll` is gone. The precommit commit now
adds an `-mattr=+avx512fp16` RUN line to the existing `fptosi-sat-scalar.ll` and
`fptosi-sat-vector-128.ll` and regenerates. That drops the NaN-specific file and
function names, and the constant qNaN/sNaN cases along with them — you were right
that they produce the same codegen as a variable operand.

With the RUN line in place the miscompile surfaces in the existing
`test_signed_i13_f16` and `test_signed_i16_f16`, so the fix commit's test delta is
45 lines confined to those two functions plus `test_signed_v8i16_v8f16`.

Two things worth flagging explicitly:

- I also added a RUN line to `fptosi-sat-vector-128.ll`, which you didn't ask for.
  The fixed-width vector forms are scalarized through this same lowering and are
  miscompiled identically, and there would otherwise be no coverage of that. I
  stopped at 128-bit deliberately: the 256- and 512-bit forms route through the
  same scalar lowering and receive the same correction, so their check blocks
  would be churn without added coverage. Happy to drop the vector file if you'd
  prefer to keep this minimal.
- Regenerating adds ~1.5k autogenerated CHECK lines, since the new RUN line
  applies to every function in both files. That is all mechanical — the
  behavioral delta is the 45 lines in the fix commit.

The baseline commit also carries FIXME markers on the three known-bad outputs,
removed again in the fix commit, per the precommit workflow in `TestingGuide.md`.

@RKSimon — your LGTM predates this. The lowering change itself is byte-identical
(only its comment was reworded), but the tests are entirely different, so it
likely warrants another look.


https://github.com/llvm/llvm-project/pull/210556