[PATCH] D136514: [AA][Intrinsics] Add separate_storage assumptions.

[David Goldblatt via Phabricator via llvm-commits]

davidtgoldblatt marked 2 inline comments as done.
davidtgoldblatt added inline comments.


================
Comment at: llvm/lib/Analysis/SeparateStorageAliasAnalysis.cpp:58
+
+  for (auto &AssumeVH : AC.assumptions()) {
+    if (!AssumeVH)
----------------
nikic wrote:
> Hm, I wonder if we can use assumptionsFor() here?
I think eventually we should be able to, but there's two things that make this tricky:
- I think the AssumptionCache assumes that the only value that matters for `assumptionsFor` is the first one (because of the param attribute background?), which would introduce a weird asymmetry in how we can use it.
- We care about the comparison after a getUnderlyingObject call, but the AssumptionCache doesn't trace through GEPs in findAffectedValues.
(plausibly other issues; I checked that naive attempts at assumptionsFor didn't work, but didn't dig as much into the why).

I think the right thing to do eventually is to rewrite the hints during InstCombine (which should fix issue 2 as well as just speeding things up), but figured it didn't make sense to add extra complication to the AssumptionCache to fix 1 until that was done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136514