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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Thanks for the quick update.

You can (for now) remove the changes that are not in Attributor.{h,cpp} as they all relate to an IR attribute (which we might want to have but that is a different beast).
Other than that there is the query interface that needs to change to provide the positions the user is interested in (see below).

Once these two changes are done we can merge the skeleton code in.



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:1695
+  /// Returns true if instruction is known reachable.
+  bool isKnownReachability() const { return getKnown(); }
+
----------------
I would replace the two methods above by:
```
AAReachability::isAssumedReachable(const IRPosition &From, const IRPosition &To)
AAReachability::isKnownReachable(const IRPosition &From, const IRPosition &To)

```
with their implementation (initially always `return true`) in AAReachabilityImpl or AAReachabilityFunction.

The idea is that users provide the two positions they are interested in and the class determines (and caches) reachability.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2074
+  /// See AbstractAttribute::trackStatistics()
+  void trackStatistics() const override { STATS_DECLTRACK_CS_ATTR(reachable); }
+};
----------------
If you keep two classes only one needs the `trackStatistics`


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2082
+  void trackStatistics() const override {
+    STATS_DECLTRACK_CSARG_ATTR(Reachability)
+  }
----------------
For functions we use:
`STATS_DECLTRACK_FN_ATTR`


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

https://reviews.llvm.org/D70233