[PATCH] D85314: [IR] Add FixedPointBuilder.

Wed Aug 19 10:28:31 PDT 2020

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: bjope.

In D85314#2225729 <https://reviews.llvm.org/D85314#2225729>, @ebevhan wrote:

> In D85314#2224845 <https://reviews.llvm.org/D85314#2224845>, @rjmccall wrote:
>
>> I'm assuming that you've extracted this code out of the corresponded Clang code and that there's nothing functionally wrong with it.  The interface looks fine.  Can we add LLVM tests for this, or that too difficult to fit into the existing test infrastructure, and we should just wait to test it by moving Clang to use it?
>
> The conversion code is lifted straight from there, but I did fold the signed/unsigned changes from D82663 <https://reviews.llvm.org/D82663>, so the emission of binops is not strictly the same as Clang currently. Should I split this up?

No, I have no objection to that, I think.

> I was thinking it would be better to rely on Clang for the tests here since the amount of duplication it would cause to test in LLVM as well doesn't seem very constructive.

Okay.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85314