[PATCH] D59648: [BasicAliasAnalysis] Fix computation for arrays > half of address space

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1453
+    if (V2Size != LocationSize::unknown()) {
+      if ((GEP1BaseOffset.isNonNegative() ? GEP1BaseOffset
+                                          : GEP1BaseOffset + ASSize)
----------------
You use:

  (GEP1BaseOffset.isNonNegative() ? GEP1BaseOffset : GEP1BaseOffset + ASSize)

or it's negation, several times here. Can we add a variable, or lambda function, etc. to make this repetition unnecessary?



================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1518
+
+        // Compute the maximum value of the offset for this index.
+        MaxVarOffset += (SignKnownZero ? ~Known.Zero : Known.One)
----------------
Can you please document better what's going on here?


================
Comment at: test/Analysis/BasicAA/cs-cs.ll:276
 ; CHECK: NoModRef:  Ptr: i8* %q <->  call void @an_inaccessiblememonly_func()
-; CHECK: NoModRef:  Ptr: i8* %p <->  call void @an_inaccessibleorargmemonly_func(i8* %q)
+; CHECK: Both ModRef:  Ptr: i8* %p <->  call void @an_inaccessibleorargmemonly_func(i8* %q)
 ; CHECK: Both ModRef (MustAlias):  Ptr: i8* %q <->  call void @an_inaccessibleorargmemonly_func(i8* %q)
----------------
Why did this change?

The change doesn't look wrong, but it's also unrelated to the half-the-address-space issue (AFAIKT, this test has 32-bit pointers), so it will be useful to understand what fixed this.



Repository:
  rL LLVM

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

https://reviews.llvm.org/D59648