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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added reviewers: nikic, lebedev.ri, reames, fhahn.
fhahn added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:668
+            return RK.AttrKind == Attribute::NonNull ||
+                   !llvm::NullPointerIsDefined(
+                       Q.CxtI->getFunction(),
----------------
nit: is llvm:: needed?


================
Comment at: llvm/lib/IR/KnowledgeRetention.cpp:314
+      if (auto *Intr = dyn_cast<IntrinsicInst>(U.getUser()))
+        if (Intr->getIntrinsicID() == Intrinsic::assume) {
+          RetainedKnowledge RK =
----------------
There's AssumptionCache, which is used in various places that inspect assumes, including ValueTracking. Is there a reason to not use it here?

 If there is a layering problem, I think that would need a bit more thought, but ideally we would re-use the existing infrastructure to make things available as widely as possible.


================
Comment at: llvm/lib/IR/KnowledgeRetention.cpp:318
+          for (auto Attr : AttrKinds)
+            if (Attr == RK.AttrKind && DT->dominates(Intr, Ctx) && Filter(RK))
+              return true;
----------------
There's isValidAssumeForContext (in ValueTracking.h, https://llvm.org/doxygen/namespacellvm.html#af1d57d70f59588a35ebfe3c4f50fecd2), which already has a similar check (but more general and not necessarily requiring DT. It might be good to use that here instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76149