Page MenuHomePhabricator

[Fixed Point Arithmetic] Fixed Point Subtraction
ClosedPublic

Authored by leonardchan on Dec 18 2018, 11:33 AM.

Details

Summary

This patch covers subtraction between fixed point types and other fixed point types or integers, using the conversion rules described in 4.1.4 of N1169.

Diff Detail

Repository
rL LLVM

Event Timeline

leonardchan created this revision.Dec 18 2018, 11:33 AM
rjmccall added inline comments.Dec 19 2018, 8:50 PM
clang/lib/CodeGen/CGExprScalar.cpp
3397 ↗(On Diff #178744)

Please go ahead and make this a covered switch with a bunch of unimplemented cases.

leonardchan marked an inline comment as done.
rjmccall added inline comments.Jan 9 2019, 4:06 PM
clang/lib/CodeGen/CGExprScalar.cpp
3451 ↗(On Diff #180920)

Please create a separate case for the non-arithmetic operators (pointer-to-member, assignment, comma) that will obviously never end up in this function.

Otherwise, LGTM.

leonardchan marked 2 inline comments as done.
leonardchan added inline comments.
clang/lib/CodeGen/CGExprScalar.cpp
3451 ↗(On Diff #180920)

Done. Although this patch wasn't marked as ready to land, do you mean that I can commit it still with the LGTM?

I don't pay attention to "ready to land" because I assume that you're verifying that your patch actually works as promised in practice, at least as far as the tests are concerned (and presumably my review catches deeper issues). If there are substantial changes that landed after my review, then I need to re-review; otherwise, you should feel free to commit with that kind of provisional LGTM when you've satisfied the provisions.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 16 2019, 11:57 AM
This revision was automatically updated to reflect the committed changes.