[PATCH] D98718: [AA][NFC] Convert AliasResult to class containing offset for PartialAlias case.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Possibly MergeAliasResults should preserve the offset when merging two PartialAlias with the same offset?



================
Comment at: llvm/include/llvm/Analysis/AliasAnalysis.h:93
+public:
+  enum Result : uint8_t {
+    /// The two locations do not alias at all.
----------------
I'd rename this to something like Type or Kind.


================
Comment at: llvm/include/llvm/Analysis/AliasAnalysis.h:140
 /// << operator for AliasResult.
 raw_ostream &operator<<(raw_ostream &OS, AliasResult AR);
 
----------------
I think the AliasResult printing code should be adjusted to also print the PartialAlias offset. This will allow us to test this functionality using normal `-aa-eval` tests, instead of unit tests. That should be a great deal simpler when testing more complex cases.


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1139
+        AliasResult AR = AliasResult::PartialAlias;
         if (VisitedPhiBBs.empty() && VRightSize.hasValue() &&
+            Off.ule(INT32_MAX) && (Off + VRightSize.getValue()).ule(LSize)) {
----------------
Now that the offset is part of the result, I *think* the VisitedPhiBBs check is not needed anymore, but I'm not entirely sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98718