[PATCH] D76149: [AssumeBundles] Use assume bundles in isKnownNonZero

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Analysis/AssumeBundleQueries.cpp:131
+  auto *Intr = dyn_cast<IntrinsicInst>(U->getUser());
+  if (!Intr || Intr->getIntrinsicID() != Intrinsic::assume ||
+      &*Intr->args().begin() == U)
----------------
This probably could be simplified by using the stuff from PatternMatch.h, e.g. something like `match(U->getUser(), m_Intrinsic<Intrinsic::assume>(m_Specific(U))`


================
Comment at: llvm/lib/Analysis/AssumeBundleQueries.cpp:145
+    AssumptionCache *AC,
+    llvm::function_ref<bool(RetainedKnowledge, Instruction *)> Filter) {
+  if (AC) {
----------------
nit: llvm:: not needed there I think


================
Comment at: llvm/lib/Analysis/AssumeBundleQueries.cpp:149
+      IntrinsicInst *II = cast_or_null<IntrinsicInst>(Elem.Assume);
+      if (!II || Elem.Index == AssumptionCache::ExprResultIdx)
+        continue;
----------------
Can the cache contain elements that are not assumes? I think ideally it would only contain entries with valid assume calls.


================
Comment at: llvm/lib/Analysis/AssumeBundleQueries.cpp:153
+              *II, II->bundle_op_info_begin()[Elem.Index]))
+        if (llvm::is_contained(AttrKinds, RK.AttrKind) && Filter(RK, II))
+          return RK;
----------------
llvm:: not needed here I think as there is `using namespace llvm`.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:699
+    SmallVector<Attribute::AttrKind, 2> AttrKinds{Attribute::NonNull};
+    if (!llvm::NullPointerIsDefined(Q.CxtI->getFunction(),
+                                    V->getType()->getPointerAddressSpace()))
----------------
nit: llvm:: not needed I think


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76149