Page MenuHomePhabricator

[IR] Add FixedPointBuilder.
ClosedPublic

Authored by ebevhan on Aug 5 2020, 7:45 AM.

Details

Summary

This patch adds a convenience class for using FixedPointSemantics
to build fixed-point operations in IR.

RFC: http://lists.llvm.org/pipermail/llvm-dev/2020-August/144025.html

Diff Detail

Event Timeline

ebevhan created this revision.Aug 5 2020, 7:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2020, 7:45 AM
ebevhan requested review of this revision.Aug 5 2020, 7:45 AM
ebevhan updated this revision to Diff 283247.Aug 5 2020, 7:53 AM

Fixed minor mistake.

ebevhan updated this revision to Diff 283869.Aug 7 2020, 4:45 AM

Fix some formatting issues.

ebevhan edited the summary of this revision. (Show Details)Aug 7 2020, 4:48 AM

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?

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, so the emission of binops is not strictly the same as Clang currently. Should I split this up?

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.

rjmccall accepted this revision.Aug 19 2020, 10:28 AM

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, 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.

This revision is now accepted and ready to land.Aug 19 2020, 10:28 AM
This revision was landed with ongoing or failed builds.Aug 20 2020, 1:31 AM
This revision was automatically updated to reflect the committed changes.