[PATCH] D59648: [BasicAliasAnalysis] Fix computation for arrays > half of address space
Hal Finkel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 12 12:02:16 PDT 2019
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
More information about the llvm-commits
mailing list