[PATCH] D140851: [Patch 3/4]: Add cases for assume (X & Y != {0|Y})

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.
Herald added a subscriber: StephenFan.

The implementation looks basically fine. I think it would be easiest to land this if it's detached from the patch stack, with some dedicated tests -- which would probably be pretty much just these?

  declare void @llvm.assume(i1)
  
  define i32 @pow2(i32 %x) {
    %and = and i32 %x, 4
    %cmp = icmp ne i32 %and, 0
    call void @llvm.assume(i1 %cmp)
    %and2 = and i32 %x, 4
    ret i32 %and2
  }
  
  define i32 @not_pow2(i32 %x) {
    %and = and i32 %x, 3
    %cmp = icmp ne i32 %and, 0
    call void @llvm.assume(i1 %cmp)
    %and2 = and i32 %x, 3
    ret i32 %and2
  }



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:943
       break;
+    case ICmpInst::ICMP_NE:
+      // assume (v & b != 0) where b is a power of 2
----------------
Huh, I would have expected this to require `{}` due to the variable declaration.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:946
+      const APInt *BPow2;
+      if (match(Cmp, m_c_ICmp(Pred, m_c_And(m_V, m_Power2(BPow2)), m_Zero())) &&
+          !BPow2->isZero() && isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
----------------
The `c_`s here can be dropped due to canonicalization (constant on the right).


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:947
+      if (match(Cmp, m_c_ICmp(Pred, m_c_And(m_V, m_Power2(BPow2)), m_Zero())) &&
+          !BPow2->isZero() && isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
+        Known.One |= BPow2->zextOrTrunc(BitWidth);
----------------
`!BPow2->isZero()` is not necessary, `m_Power2` only matches power of twos (unlike `m_Power2OrZero`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140851