[PATCH] D41659: Implementing missing trigonometric optimizations

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 3 06:49:47 PST 2018


spatel added a comment.

In https://reviews.llvm.org/D41659#965694, @fhahn wrote:

> Thanks for your patch!
>
> Usually it is easier to review smaller patches. Splitting it up into 4 patches for the 4 different cases you implemented would make it slightly easier to reason about the 4 unrelated transformation in isolation.


Strongly agree. I understand there's a common theme here, but it will save time making redundant comments if we start with a patch for just one of these transforms.

The structure of the patch is not correct. We don't need a trig helper function because it's impossible for an integer op like mul or udiv to have an FP operand like llvm.sin.
Please have a look at these patches for the correct way to match intrinsics and libcalls:
https://reviews.llvm.org/D41322
https://reviews.llvm.org/D41283
https://reviews.llvm.org/D41389


Repository:
  rL LLVM

https://reviews.llvm.org/D41659





More information about the llvm-commits mailing list