[PATCH] D61238: [ConstantRange] Add sdiv() support

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:1018
+  if (!NegL.isEmptySet() && !NegR.isEmptySet()) {
+    // neg / neg = neg.
+    //
----------------
I'm confused, isn't `-3 / -3 = 1` ?


================
Comment at: llvm/lib/IR/ConstantRange.cpp:1030
+    // form i4 [-3, -8] the next larger number exluding -8 would actually be -3
+    // rather than -7.
+    APInt Lo = (NegL.Upper - 1).sdiv(NegR.Lower);
----------------
Is it worth the hassle? Ignoring the special case and taking the APInt behavior seems like a much easier solution. Do we have evidence we don't want that? (I did not try to understand this special case yet!)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61238