[PATCH] D142234: [LVI] Handle Intrinsic::ctlz

[Nikita Popov via Phabricator via llvm-commits]

nikic requested changes to this revision.
nikic added a comment.
This revision now requires changes to proceed.

The unit test currently fails both for correctness and optimality checks:

  /var/lib/buildkite-agent/builds/llvm-project/llvm/unittests/IR/ConstantRangeTest.cpp:137
  Value of: rangeContains(CR, APInt(BitWidth, Elem), Inputs)
    Actual: false (empty-set does not contain 0 for inputs: full-set, )
  Expected: true
  /var/lib/buildkite-agent/builds/llvm-project/llvm/unittests/IR/ConstantRangeTest.cpp:137
  Value of: rangeContains(CR, APInt(BitWidth, Elem), Inputs)
    Actual: false (empty-set does not contain -1 for inputs: full-set, )
  Expected: true
  /var/lib/buildkite-agent/builds/llvm-project/llvm/unittests/IR/ConstantRangeTest.cpp:145
  Value of: CR.isEmptySet()
    Actual: false
  Expected: true
  /var/lib/buildkite-agent/builds/llvm-project/llvm/unittests/IR/ConstantRangeTest.cpp:177
  Value of: NotPreferred(PossibleCR)
    Actual: false (Inputs = full-set, CR = full-set, BetterCR = [0,-1))
  Expected: true
  /var/lib/buildkite-agent/builds/llvm-project/llvm/unittests/IR/ConstantRangeTest.cpp:145
  Value of: CR.isEmptySet()
    Actual: false
  Expected: true
  /var/lib/buildkite-agent/builds/llvm-project/llvm/unittests/IR/ConstantRangeTest.cpp:177
  Value of: NotPreferred(PossibleCR)
    Actual: false (Inputs = [0,2), CR = full-set, BetterCR = [3,4))
  Expected: true
  /var/lib/buildkite-agent/builds/llvm-project/llvm/unittests/IR/ConstantRangeTest.cpp:177
  Value of: NotPreferred(PossibleCR)
    Actual: false (Inputs = [0,3), CR = full-set, BetterCR = [2,4))
  Expected: true
  /var/lib/buildkite-agent/builds/llvm-project/llvm/unittests/IR/ConstantRangeTest.cpp:177
  Value of: NotPreferred(PossibleCR)
    Actual: false (Inputs = [0,4), CR = full-set, BetterCR = [2,4))
  Expected: true
  /var/lib/buildkite-agent/builds/llvm-project/llvm/unittests/IR/ConstantRangeTest.cpp:177
  Value of: NotPreferred(PossibleCR)
    Actual: false (Inputs = [0,5), CR = full-set, BetterCR = [1,4))
  Expected: true
  [...]

You can run the unit tests using `ninja -Cbuild check-llvm-unit`. Or `ninja -Cbuild IRTests && build/unittests/IR/IRTests` to run just this test suite.



================
Comment at: llvm/include/llvm/ADT/APInt.h:1741
+  /// Get the leading zeroes of a value.
+  APInt ctlz() const { return APInt(BitWidth, this->countLeadingZeros()); }
+
----------------
I'd rather not have this API on APInt. Getting leading zeros as an APInt is pretty unusual.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:1684
+    // The range may become full if it *only* contains Zero.
+    return getFull();
+  }
----------------
Constant ranges are understood modulo poison. The value is in the range or it is poison. As such, if ZeroIsPoison is set, we should return a //better// range that doesn't have to account for zero input, not a worse range.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:1700
+  } else {
+    if (Lower.ugt(Upper))
+      std::swap(Lower, Upper);
----------------
Can Lower be larger than Upper for a non-wrapped set?


================
Comment at: llvm/test/Analysis/LazyValueAnalysis/lvi-for-ctlz.ll:1
+; RUN: opt < %s -passes=jump-threading -print-lvi-after-jump-threading -disable-output 2>&1 | FileCheck %s
+
----------------
Having an IR test is fine, but please do not test LVI debug output. Just check the resulting IR change using update_test_checks.py.


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

https://reviews.llvm.org/D142234