[PATCH] D74888: [Attributor] Use knowledge retained in llvm.assume (operand bundles)
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 22 22:52:32 PST 2020
jdoerfert marked 2 inline comments as done.
jdoerfert added inline comments.
================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:681
bool IRPosition::hasAttr(ArrayRef<Attribute::AttrKind> AKs,
- bool IgnoreSubsumingPositions) const {
+ bool IgnoreSubsumingPositions, Attributor *A) const {
SmallVector<Attribute, 4> Attrs;
----------------
uenoku wrote:
> I think using `nullptr` as a flag to indicate subsuming positions seems a bit difficult to read.
> Could you simply add some boolean flag and replace the argument with a reference?
I'm not sure I understand. If `A` is null, you don't get any assume handling. If `A` is not null, you get assume handling but only for the current position. not any subsuming one.
================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:748-753
+ for (const Use &U : AssociatedValue.uses())
+ if (IntrinsicInst *II = dyn_cast_or_null<IntrinsicInst>(U.getUser()))
+ if (II->getIntrinsicID() == Intrinsic::assume && !II->isArgOperand(&U))
+ if (hasAttributeInAssume(*II, &AssociatedValue, AttrName, ArgValPtr))
+ if (!ArgValPtr || ArgVal > 0)
+ AssumeAttrMap[II] = Attribute::get(Ctx, AK, ArgVal);
----------------
uenoku wrote:
> Can't we share `AssumeAttrMap` with other AAs whose associated values are the same?
Yes, that is the plan. In KnowledgeRetention.{h/cpp} there is a TODO that says we should provide an API to get all information from an `llvm.assume` in a map. Once that is in place, the Attributor should be the one doing the querying (once) and then caching the information. Does that make sense?
I hoped @Tyker will look into that API at some point.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74888/new/
https://reviews.llvm.org/D74888
More information about the llvm-commits
mailing list