[PATCH] D93974: [ValueTracking] Safe assumption context for args

[Gil Rapaport via Phabricator via llvm-commits]

gilr added a comment.

In D93974#2489195 <https://reviews.llvm.org/D93974#2489195>, @jdoerfert wrote:

> FWIW, I agree with @nikic, we should not put this logic here. There are two problems:
>
> 1. We compute something we might not need.
> 2. We do it only in value tracking.

Thanks for taking a look, Johannes!

> I'd prefer the following, though the nullptr proposal seems fine too:
> a) For arguments use the first instruction in the entry as context, this is trivial and correct.

Agreed. Will limit this patch for this low-hanging fruit. Handling null contexts in isValidAssumptionForContext() is indeed more general, but also seems to have greater potential for causing trivial simplification of ephemeral values. If it works out it would replace the safe context for arguments.

> b) When the context is used, e.g., to look for assumption, allow to some exploration of surrounding instructions.

Not sure I see how (beyond the existing extension to reachable instructions). Since isValidAssumeForContext() can't tell why its context was chosen it must assume the context might be protecting an ephemeral from simplification, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93974