[PATCH] D70233: [WIP][NOT FOR COMMIT][Attributor] AAReachability Attribute

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Thanks for putting this in the open! I hope we can improve this and get it in!

---

Here are some high-level comments I had after looking over the patch:

I don't know how we would handle "reachability" as an actual IR-attribute. For now you might want to strip this to only the Attributor parts which can deal with it as an "abstract" attribute, like liveness or heaptostack.

We should start with `AAReachabilityFunction` only which answers the following questions:
 Given two IRPositions, is one assumed/known reachable from another.
To this end, `AAReachability` will need to expose these query functions, e.g.,:
 `AAReachability::isAssumedReachable(const IRPosition &From, const IRPosition &To)`
 `AAReachability::isKnownReachable(const IRPosition &From, const IRPosition &To)`



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:1687
+struct AAReachability
+    : public IRAttribute<Attribute::Reachability, AbstractAttribute> {
+  AAReachability(const IRPosition &IRP) : IRAttribute(IRP) {}
----------------
You don't need this to be an IRAttribute, an AbstractAttribute is sufficient. You will need to store the IRPosition in the class explicitly though.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:5571
+      getOrCreateAAFor<AAReachability>(ArgPos);
+
       // Every argument with pointer type might be marked dereferenceable.
----------------
I don't know if we need to create `AAReachability` or if we only want to "derive" it whenever another attribute requests the information. Since for now we cannot manifest it, I would not try to create it eagerly but only on demand. That means, remove these two `getOrCreateAAFor` calls but we later add `getOrCreateAAFor<AAReachability>` as part of the deduction (via `updateImpl`) in other attributes.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:5923
 CREATE_VALUE_ABSTRACT_ATTRIBUTE_FOR_POSITION(AANoAlias)
+CREATE_VALUE_ABSTRACT_ATTRIBUTE_FOR_POSITION(AAReachability)
 CREATE_VALUE_ABSTRACT_ATTRIBUTE_FOR_POSITION(AADereferenceable)
----------------
We should start by a function only abstract attribute (like `AAHeapToStack`).
The macro for those is `CREATE_FUNCTION_ONLY_ABSTRACT_ATTRIBUTE_FOR_POSITION`
and you only need the `AAReachabilityFunction` derivation of `AAReachability`.
(See the main comment).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70233