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

[Daniil Fukalov via Phabricator via llvm-commits]

dfukalov added a comment.

In D98718#2635171 <https://reviews.llvm.org/D98718#2635171>, @nikic wrote:

> I do see some impact from this patch: https://llvm-compile-time-tracker.com/compare.php?from=be92dcb9f679a23329d360bbc5533acc5f808d9c&to=1f10b8ef5f0243ac0a2679d6f334275471bff316&stat=instructions

I see there you experimented with 8 bits for `Result` and zero init for Offset field - I agree it's a good ideas.



================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1582
+    // Cache contains sorted {V1,V2} pairs.
+    Entry.Result.swap(Swapped);
     return Entry.Result;
----------------
nikic wrote:
> Doesn't this swap the cached result? I think the intention here was to only return a swapped result, no?
Yes, you're right, I didn't notice it modifies cache. Thanks!


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1604
+  // Cache contains sorted {V1,V2} pairs.
+  Entry.Result.swap(Swapped);
   Entry.NumAssumptionUses = -1;
----------------
nikic wrote:
> I think we shouldn't be swapping here, as we computed the result already in swapped order.
`Result` we've just put into cache is computed by `aliasCheckRecursive(V1, ..., V2)` with original order, but `Entry` was found as `AAQI.AliasCache.find(Locs)` for sorted pair. So here we should save the `Swapped` result into cache.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98718