[PATCH] D104854: Introduce intrinsic llvm.isnan

[Siva Chandra via Phabricator via llvm-commits]

sivachandra added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22193
+                        DAG.getConstant(0x45, DL, MVT::i8));
+
+  return DAG.getSetCC(DL, ResultVT, Extract, DAG.getConstant(1, DL, MVT::i8),
----------------
While I do not understand the code mechanics of this patch, I am mostly in agreement with the general direction of this patch. However, it has lead to a change in behavior wrt 80-bit x86 floating point numbers. Unlike the 32-bit and 64-bit floating point numbers, 80-bit numbers have an additional class of "Unsupported Numbers". Those numbers were previously treated as NaNs. Since this change uses the `fxam` instruction to classify the input number, that is not the case any more as the `fxam` instruction distinguishes between unsupported numbers and NaNs. So, to restore the previous behavior, can we extend this patch to treat unsupported numbers as NaNs? At a high level, what I am effectively saying is that we should implement `isnan` this way:

```
bool isnan(long double x) {
  uint16_t status;
  __asm__ __volatile__("fldt %0" : : "m"(x));
  __asm__ __volatile__("fxam");
  __asm__ __volatile__("fnstsw %0": "=m"(status):);
  uint16_t c0c2c3 = (status >> 8) & 0x45;
  return c0c2c3 <= 1; // This patch seems to be only doing c0c2c3 == 1 check.
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104854/new/

https://reviews.llvm.org/D104854