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

Tue Jan 5 12:05:02 PST 2021

nikic added a comment.

In D93974#2479403 <https://reviews.llvm.org/D93974#2479403>, @gilr wrote:

> Sorry for the delay @nikic.
>
> In D93974#2476165 <https://reviews.llvm.org/D93974#2476165>, @nikic wrote:
>
>> Am I understanding correctly that this tries to use the last instruction in the entry block rather than the first one to avoid triggering the ephemeral value check, in case the first instruction is part of an assumption?
>
> Yes, the first instruction is fine except for an assume which appears (along with its ephemeral values) as the first thing in the function, which in case of an argument seems quite likely.
>
>> In any case, I don't think it's appropriate to perform a full block scan to determine the context instruction. safeCxtI() should be cheap (as in O(1)).
>
> An unbounded scan of the entry block might indeed be too much even if applied only to arguments. The scan can perhaps be limited to some small number of instructions (5 seems like the minimum to cover most patterns in computeKnownBits()) which gets reset if an assume is encountered.

Another possibility would be to leave the context instruction at nullptr, and instead adjust isValidAssumeForContext to accept a nullptr CxtI in which case only instructions that are must-exec from the function entry are considered. Advantage is that it only incurs a cost if there is a potentially relevant assume.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93974