[PATCH] D117180: [BasicAliasAnalysis] Switch from isMallocOrCallocLikeFn to onlyAccessesInaccessibleMemory

Tue Jan 18 00:06:45 PST 2022

nikic added a comment.

In D117180#3249638 <https://reviews.llvm.org/D117180#3249638>, @reames wrote:

> @Bryce-MW It's a bit more complicated than just checking existing test cases.  From memory, this explains some code in both AliasAnalysis.cpp and BasicAliasAnalysis.cpp which I'd thought odd when looking through it this morning.  I think the attribute change is probably worthwhile, but it will require both an RFC and a number of changes to existing code.
>
> I can drive that.  I'll wait a day or two to see if anyone remembers a strong reason for the current semantic, then work up some patches and an RFC to llvm-dev.

Not sure I understand what is being suggested here. I do think that using `inaccessiblememonly` for calloc is fine, as the memory is inaccessible at the time of the call, and becomes accessible after it returns. calloc isn't really different from other allocation functions here.

I think the problematic interaction with DSE is the fact that MSSA does not model object lifetime, so if a new object comes into existence (via alloca, malloc ... or calloc) it will be liveOnEntry. The accepted way to handle that right now is to check that the defining access is liveOnEntry and the underlying object is an alloca or allocation.

I've put up D117543 <https://reviews.llvm.org/D117543> to show that DSE would be perfectly capable of handle inaccessiblememonly calloc, it just requires a slightly different implementation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117180