[PATCH] D109844: [DSE] Track earliest escape, use for loads in isReadClobber.

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/test/Transforms/DeadStoreElimination/captures-before-load.ll:127
   %in.lv.1 = load i32* , i32** %in.ptr, align 2
   call void @escape_writeonly(i32* %a)
   %in.lv.2 = load i32 , i32* %in.lv.1, align 2
----------------
fhahn wrote:
> nikic wrote:
> > To check my understanding, it would be fine to optimize this case right? That is, what we actually care about is not an escape before the load (`%in.lv.2`), but before the instruction producing the loaded value (`%in.lv.1`). That is, our context instruction could be `ReadUO` rather than `UseInst`.
> > 
> > If that's correct, then we might be able to sink this new functionality into AA proper (e.g. via a BatchAA flag for more expensive capture analysis). I previously thought this wasn't possible because alias() does not have a context instruction, but with this we don't actually need one.
> We should be able to use `ReadUO` , once we loaded the object the escapes cannot change it. 
> 
> > then we might be able to sink this new functionality into AA proper (e.g. via a BatchAA flag for more expensive capture analysis)
> 
> Sounds good. Do you think it would be worth to also move the caching of the escapes to BatchAA? Are there any users of BatchAA you would be interested in (other than DSE)? 
A quick prototype for moving to AA: https://github.com/llvm/llvm-project/commit/8e3a5699f206ae36c513a01f953d758ee93e7f54

This has some compile-time impact, not sure how much is just from the dropped LoopInfo: https://llvm-compile-time-tracker.com/compare.php?from=1e3c6fc7cb9d2ee6a5328881f95d6643afeadbff&to=8e3a5699f206ae36c513a01f953d758ee93e7f54&stat=instructions (Note that this implicitly enables the call capture part of the change.)

I think we need some different way to expose the capture cache as a dependency though. I don't really like shoving more datastructures directly in AAQueryInfo, especially conditionally used ones. It would be nicer to pass in some CaptureInfo object that can then be backed either by a simple or by a reachability based implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109844