[PATCH] D60522: [KnownBits] Add computeForAddCarry()

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Support/KnownBits.cpp:54
+
+KnownBits KnownBits::computeForAddSub(bool Add, bool NSW,
+                                      const KnownBits &LHS, KnownBits RHS) {
----------------
nikic wrote:
> lebedev.ri wrote:
> > All these `bool` params are potential bugs, and really affect readability.
> > But i guess that would be too intrusive for this header.
> We might want to start a separate file outside Support (I'm thinking `llvm/Analysis/KnownBitsOps.h` and `lib/Analysis/KnownBitsOps.cpp`) that can depend on things like binop opcodes and obo flags and allow sharing more of the KnownBits code between ValueTracking and SDAG. The existing KnownBits header mostly has only primitive operations and is quite cumbersome to edit (because it rebuilds half of LLVM).
Sorry, there was another phrase between those two i posted:
> All these bool params are potential bugs, and really affect readability.
"i'd prefer to pass them as some enum fields, e.g. the ones used elsewhere in IR for IR instructions and wrap flags"
>But i guess that would be too intrusive for this header.
But yes, i guess it will be best to reinvent the wheel here, and add whole new enum.


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

https://reviews.llvm.org/D60522