[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 29 03:44:40 PDT 2020


xazax.hun accepted this revision.
xazax.hun added a comment.

@szelethus
The patch looks good to me and I find it a welcome change that should make things easier to understand and maybe even a bit more efficient, I hope this won't break ObjC :) My discussion with Artem is orthogonal to this change.

In D82598#2117432 <https://reviews.llvm.org/D82598#2117432>, @NoQ wrote:

> Well, formally it isn't.


Right. Sorry for being unclear there.

> I guess your point is that live expressions analysis could be replaced with imperative cleanup requests from, say, `ExprEngine` to the `Environment`. I.e., "We've just finished evaluating the if-statement, now we should actively tell the Environment to remove the condition expression".

Yes. This is what I am proposing and this is what we did with the lifetime analysis. Unfortunately, I did not have time yet to identify all the cleanup points and I do agree that this is not a trivial task but I felt that this is much more lightweight in terms of memory and computation.

> I guess it could totally work that way, but it'd be pretty hard to not forget such cleanups.

I agree.

> Given that we'd also barely ever notice that we forgot one of those, i'm very much in favor of having liveness analysis instead, that would declaratively describe which expressions are live when, so that to automatically guarantee that we always only track what's necessary and never snowball our state with dead expressions.

Fair point. I do not remember any problem with the current liveness analysis so I guess if something is not broken do not fix it. We also don't have any evidence of this being a bottleneck.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598





More information about the cfe-commits mailing list