[PATCH] D104432: [Attributor] Introduce AAPointerInfo

[Kuter Dinel via Phabricator via llvm-commits]

kuter added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:1012-1013
+
+  /// See AAPointerInfo::forallInterfearingAccesses.
+  bool forallInterfearingAccesses(
+      Instruction &I,
----------------
This function is going to be called from the `updateImpl` method of a attribute.
Maybe it would make sense to make it a little faster at least in some common cases or maybe some caching ?



================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:1034
+    // Now that we have an offset and size, find all overlapping ones and use
+    // the callback on the accesses.
+    for (auto &It : AccessBins) {
----------------
Maybe it would be worth handling the case where the access is always aligned ?
like `int array[100];`

Maybe we can have a flag inside the state that tells us that all the load/store instructions that we looked at have aligned access.  
Then we can just use the offset (maybe through a hashmap lookup) to find any interfering access.

Or maybe  if we have  sorted set like std::set (sorted by offset). We can limit where we end our scan.
We would only scan [0, OAS.getOffset() + OAS.getSize()].

Or even better if we have  maximum access size for all access
We can just scan just scan the range [OAS.getOffset() - MaxSize, OAS.getOffset() + OAS.getSize()].




================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:1102
+            RAcc.getContent(), CB, *this, UsedAssumedInformation);
+        AAPointerInfo::Access TranslatedAcc(&CB, Content, RAcc.getKind(),
+                                            RAcc.getType());
----------------
Maybe we can use  `addAccess` here  ? 


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:1203
+            !GEP->hasAllConstantIndices()) {
+          UsrOI.Offset = OffsetAndSize::Unknown;
+        } else {
----------------
Maybe an early return here would be easier to read ?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104432