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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/IR/KnowledgeRetention.cpp:314
+      if (auto *Intr = dyn_cast<IntrinsicInst>(U.getUser()))
+        if (Intr->getIntrinsicID() == Intrinsic::assume) {
+          RetainedKnowledge RK =
----------------
Tyker wrote:
> fhahn wrote:
> > 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.
> isValidAssumeForContext is now used and i saw the improvement in a test case.
> 
> for AssumptionCache. it could be used here but i think it would be costlier to use it than what we currently have.
> we are working with knowledge in operand bundles, the assume is always the direct user of the value it knows something about (which is not true for boolean assumes). so we can iterate over all uses (non-recursively) to find all candidate llvm.assume. looking up the knowledge in the assume is cheaper when we have the the operand number which is only available when we lookup from the uses.
> for AssumptionCache. it could be used here but i think it would be costlier to use it than what we currently have.
 I am not sure how using assumptionCache would be more costly, assuming it is already populated?

> we are working with knowledge in operand bundles, the assume is always the direct user of the value it knows something about (which is not true for boolean assumes).

But you still have to check all uses of each value (of which many may not be assumptions), to find the available assumptions IIUC. By using AssumptionCache, you can skip the traversal of the use list, which should be quite beneficial, assuming most values do not have associated assumptions.



================
Comment at: llvm/lib/IR/KnowledgeRetention.cpp:302
+                                  ArrayRef<Attribute::AttrKind> AttrKinds) {
+  if (auto *Intr = dyn_cast<IntrinsicInst>(U->getUser()))
+    if (Intr->getIntrinsicID() == Intrinsic::assume) {
----------------
This could be an early exit (like `auto *II = dyn_cast....; if (!II) return none()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76149