[PATCH] D146905: [IR] Add llvm.tan.* intrinsic

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

I voiced this before <https://discourse.llvm.org/t/math-intrinsics/67192/6?u=jdoerfert> and while I will repeat it here I won't block this patch all by myself:

This is a bad idea.

We are playing whack-a-mole and assuming we don't want to commit to all of them (math intrinsics), this is always going to be a source of "weird behavior", to say the least.
Even if we had all of them there are still unsolved problems as soon as you leave the space of systems with a libm that is linked in late since not all intrinsics lower to target instructions but rather libm calls.

What we should do is to remove all math intrinsics and instead work on the math functions themselves, as we do with other library functions (malloc, free, ...).
A math intrinsic is literally a regular call to the corresponding math function with a `no_errno` flag attached to it (and the type encoded in the name):
We probably even have all the attributes today to express that:

  declare double @tan(double) memory(readonly)

[Not to mention that this makes tan, which is represented as target independent math intrinsic, special only for X86...]

Now there have been arguments for intrinsics <https://discourse.llvm.org/t/math-intrinsics/67192/8?u=jdoerfert>, e.g., lib call matching is bad. I don't disagree, but we can fix that.
It was also raised that we treat intrinsics as cheap and calls as expensive in heuristics, though, that is just not right to begin with.
A math call is the same if we call the math function as part of the lowering anyway, no matter if it was a call or intrinsic call in the IR.
That said, we can make smarter choices for heuristics fairly easily, e.g., a `readnone` call is probably not going to be too expensive.

All that said, I think we should recognize math functions and annotate them, but just not as intrinsics. Maybe something like this:

  define double @tan(double) libm(tan,double))
  define float @tanf(float) libm(tan,float))
  ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146905