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

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D117180#3250435 <https://reviews.llvm.org/D117180#3250435>, @nikic wrote:

> In D117180#3249638 <https://reviews.llvm.org/D117180#3249638>, @reames wrote:
>
>> 
>
> 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 agree that calloc behaves like malloc in this respect.  The basic question I see being posed here is whether inaccessiblememonly is legal on any allocation routine.

I was originally thinking we needed to change the definition to explicitly allow this, but it sounds like you think this was always allowed.  I put up a langref change (https://reviews.llvm.org/D117571) to make that really explicit.

> 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 handling inaccessiblememonly calloc, it just requires a slightly different implementation.

I think I agree.  I was originally thinking we'd need AA changes for the definition point (what you call "coming into existence"), but looking at how we handle allocas, you're proposal is more consistent than mine.

Is your change ready for review?  Or do you need to do something else first?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117180