[PATCH] D25386: [LVI] Try to document the complexities of context instructions

[Anna Thomas via llvm-commits]

anna added inline comments.


================
Comment at: include/llvm/Analysis/LazyValueInfo.h:77
+  /// through a block.  The simplest example is we may encounter a function
+  /// which throws if it's argument is non-zero.  For an context before that
+  /// function call, we have to assume the value might be non-zero, but after
----------------
s/an context/a context/


================
Comment at: include/llvm/Analysis/LazyValueInfo.h:84
+  /// special intrinsics with related semantics: assume, and guards.
+  ///    The optional context instruction provides a means to query these
+  /// facts.  We also allow context instructions which are not within the block
----------------
reames wrote:
> sanjoy wrote:
> > Have you tried restructuring the description this way:
> > 
> > ```
> > The LVI query predicates accept a CFG element (Block, Edge) and an optional context instruction.  When a context instruction is present ... (semantics of the context instruction).  If it isn't present, we pretend the context instruction is the beginning of the block / somewhere "inside" the edge.
> > ```
> > 
> Hm, you repharsing of the missing context case is interesting.  In particular, we might be able to implement the semantics you suggest and get slightly more precise results than we do today.  This *isn't* our semantic today, but it would be a completely reasonable one and I don't think it would break any existing users.  
> 
> We'd have to pick beginning or end consistently.  End seems like it's more powerful, what do you think?  (i.e. for an invoke's normal edge, we'd want the context to be the first non-phi instruction in the normal block, not the invoke itself.)
How is the value chosen currently when the context instruction is not specified? Is this correct:
1. in the case of single Block -- value of Y depends on all instructions in BB? One of these instructions might as well be the context instruction, but it wasnt specified.
2. for Edge (start BB, end BB) -- value of Y depends on instructions from first inst in start BB upto last inst in end BB. 


================
Comment at: lib/Analysis/LazyValueInfo.cpp:965
+// the solve infrastructure, it is used to find assumed facts which are true
+// about intermediate values at their point of use.  This is result is safe to
+// cache since the def-use graph/cfg is fixed for all queuries.  For an
----------------
extra 'is' - 'This result is safe to'


https://reviews.llvm.org/D25386