[PATCH] D69321: [LVI][CVP] LazyValueInfoImpl::solveBlockValueBinaryOp(): use no-wrap flags from `add` op

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 23 01:05:37 PDT 2019


nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM

Thanks for looking into this! I think we should definitely do this change as the functionality already exists, but based on the numbers, it probably doesn't make sense to invest in NoWrap implementations of other operators unless we also need them for something else (SCEV was the original motivation here).



================
Comment at: llvm/include/llvm/IR/ConstantRange.h:330
+  /// Return a new range representing the possible values resulting
+  /// from an application of the specified overflowing binary operator to an
+  /// left hand side of this range and a right hand side of \p Other given
----------------
nit: an left hand side -> a left hand side


================
Comment at: llvm/include/llvm/IR/ConstantRange.h:335
+                                    unsigned NoWrapKind,
+                                    const ConstantRange &Other) const;
+
----------------
I'd swap the last two arguments here to match the order of makeGuaranteedNoWrapRegion and addWithNoWrap, which both have NoWrapFlags after the range.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69321





More information about the llvm-commits mailing list