arsenm added a comment.

In D140294#4007073 <https://reviews.llvm.org/D140294#4007073>, @sepavloff wrote:

> This change can have negative consequences in some cases. Some targets have 
> dedicated instruction to test FP class and often this instruction is faster 
> than arithmetic operations. Replacement of one operation with two arithmetic 
> and two logic plus cost of FP constant materialization does not provide any 
> visible benefit.

Not sure what you're talking about, there's no class call here. Besides, 
optimal codegen is not clang's concern

> One of advantages that llvm.is_fpclass provides is the possibility for a 
> target to use FCLASS instruction if it is available. Besides, llvm.is_fpclass 
> incapsulates semantics of a class check, which is not identical to compare 
> operations. Keeping the class checks in IR can facilitate FP-specific 
> optimizations which provide different handing depending on possible FP 
> classes. Fast math is one example, denormals is another.

It's almost but not quite the same. The middle end can swap back to fcmp as 
long as we don't have FP exceptions to worry about (in the denormal case we can 
reform compares against 0/smallest_normal depending on the known denormal 
mode). I have patches ready to swap between all of these cases

> I would propose to generate llvm.is_fpclass for all classification intrinsics 
> (see D137811 <https://reviews.llvm.org/D137811>). Targets then may lower the 
> intrinsic in a way most suitable way depending on ISA.

Right, I think clang emitting is.fpclass and letting the middle and and backend 
fold back into fcmp as legal is a reasonable strategy. In cases surrounding 
denormals we just need to know the floating point mode. The possible downside 
to always emitting class upfront in clang is you immediately lose fast math 
flags (I'm not entirely sure having fast math flags on fcmp is a good idea 
though)


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

https://reviews.llvm.org/D140294

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to