[PATCH] D73679: [MemoryBuiltins] Determine the size of a global w/o initializer

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv marked an inline comment as done.
george.burgess.iv added a comment.

> So there are existing test with 0 sized objects that would fail if we do not have the Size != 0 check but I'll add a few more to nail down what we expect as soon as I find the time. It might take a week or two though.

SGTM -- thanks!



================
Comment at: llvm/test/Analysis/BasicAA/bug.23540.ll:11
 ; CHECK-LABEL: f
-; CHECK: MayAlias: i32* %arrayidx, i32* %arrayidx6
+; CHECK: PartialAlias: i32* %arrayidx, i32* %arrayidx6
 define void @f() {
----------------
jdoerfert wrote:
> george.burgess.iv wrote:
> > Can you expand a bit on why PartialAlias is the correct answer here, please?
> > 
> > I'd naively expect this to be either MustAlias (both pointers are to a 32 bit object, and there're only 32 bits in this allocation; derefing anything else is UB), or MayAlias (there're only 32 bits here, but maaaaybe you're pointing to `((int*)&c + 1)`, and that's OK.)
> My reasoning is that `arrayidx6` points right after `c` and `arrayidx` points somewhere based on `c`. There are no accesses so the alias query is done with an unspecified size on the access part which should mean the two accesses can very well overlap. 
I don't quite agree with the unspecified size part: if the access size were entirely unknown, we could only correctly return `PartialAlias` in cases with provably identical object offsets, since `PartialAlias` *guarantees* the two accesses will overlap somehow.

Experimentally, we report `NoAlias` for `(%b, %c)` in:
```
define void @f() {
  %a = alloca i32
  %b = bitcast i32* %a to i8*
  %c = getelementptr i8, i8* %b, i64 1
  ret void
}
```

So I think the sizes here should be 4 bytes. If that's the case, `MustAlias` seems like the optimal answer, which means that `PartialAlias` is also correct.  `sizeof(access) == sizeof(underlying_object)` in both cases means there's really no room for nonzero object offsets. :)

If this sounds correct, please update the newly-added comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73679