[PATCH] D137021: [ValueTracking] haveNoCommonBitsSet - add assumption cache handling (PR58624)

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Not sure this is useful. The test case from the issue is over-reduced, the actual motivating case would have to handle a dominating condition, not an assume.



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:313
+  if (AC && CxtI) {
+    for (auto &AssumeVH : AC->assumptions()) {
+      if (!AssumeVH)
----------------
Shouldn't we be lookup up the assumptions for either LHS or RHS here, rather than all of them?


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:339
+          match(Src, m_Not(m_c_And(m_Specific(LHS), m_Specific(RHS)))))
+        return true;
+    }
----------------
You're missing a context validity check. Currently you're using assumes that don't dominate the context instruction.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:3800
+  if (llvm::haveNoCommonBitsSet(Op0, Op1, DL, &AC, &I, &DT))
+    return BinaryOperator::CreateOr(Op0, Op1);
+
----------------
Is this useful independently of the assume case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137021