[LLVMdev] [PATCH][RFC]: Add fmin/fmax intrinsics

Chandler Carruth chandlerc at google.com
Thu Oct 9 16:55:51 PDT 2014


Matt, could you split up these patches significantly?

I'd also suggest a new review thread, maybe using Phab, as these are quite
large. (well, the first one is quite large. the second one is trivial..)

I would suggest at least 3 splits:

1) change/add common APIs needed to implement stuff. This includes changces
to APFloat, Constant, etc. Feel free to do one super tiny change per API if
you can, or bundle them if they're all related. Where possible, please
include a unittest for the basic API coverage.

2) add the IR-level intrinsic without any lowering. You should still be
able to test it entirely at the IR level

3) add lowering to a DAG node and handling in at least one backend. you
could include multiple backensd here if it doesn't grow the patch too much,
or split subsequent backends into a separate patch.


I would like to get all of this into the tree, but under the IEEE names.
Given the current state of the world, those names seem the least likely to
cause folks to become confused as to their semantics. The C names here are
deeply tainted because things like 'fabs' just used 'f' to signify the type
not the semantics of the operation, and so became 'abs' in C++ and
elsewhere.

On Thu, Oct 9, 2014 at 4:49 PM, Chandler Carruth <chandlerc at google.com>
wrote:

>
> On Thu, Sep 18, 2014 at 2:54 PM, Matt Arsenault <arsenm2 at gmail.com> wrote:
>
>> Updated patches that rename the intrinsics to minnum / maxnum, though the
>> ISD names are still FMIN / FMAX
>
>
> Can you rename the ISD nodes to be consistent? Looking at patches.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141009/076996ef/attachment.html>


More information about the llvm-dev mailing list