[PATCH] D113414: [X86] Custom lowering of llvm.is_fpclass for x86_fp80

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:721
     setOperationAction(ISD::LLRINT, MVT::f80, Custom);
+    setOperationAction(ISD::IS_FPCLASS, MVT::f80, Custom);
 
----------------
sepavloff wrote:
> pengfei wrote:
> > sepavloff wrote:
> > > pengfei wrote:
> > > > Add f32/f64 too?
> > > There is concern that FXAM can be slow (https://reviews.llvm.org/D104853#2839746). So it is used only for f80, where x87 anyway is used. For other types a default lowering is used, which use usual float operations or integer arithmetic to make the tests. The resulted code for x86 is presented in D112025 in the test.
> > Given that and we still need to distinguish sNaN with qNaN. Why don't we use integer arithmetic for `f80` too? We just need to check the high 32 bits for all classes expect fcZero and fcInf.
> There is no `i80`, so doing integer arithmetic on `f80` is complicated. Existing default lowering simply crashes on `f80`. Also for many classes, like `fcZero`, `fcSubnormal`, `fcInf` or `fcSNan` we need to analyze all three words of `f80`. One instruction `FXAM` does all the job, and seems faster even if its latency is high.
I mean we still customize `f80` here, but enrich the code for checking S/QNAN to more classes by loading bits [79:48] rather than [63:32]. We can easily check whether the remainding 48 bits are zero for `fcZero` and `fcInf`.
Not only for performance, but also we should lower it when `f80` is used without `x87` enabled. Although it's rare of such case, it's supported by compiler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113414