[PATCH] D44748: Track whether the size of a MemoryLocation is precise

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added inline comments.


================
Comment at: include/llvm/Analysis/MemoryLocation.h:35
+// it contains, N, is 'precise'. Precise, in this context, means that we know
+// that the MemoryLocation we're referencing has to be at least N bytes large.
+//
----------------
reames wrote:
> george.burgess.iv wrote:
> > reames wrote:
> > > george.burgess.iv wrote:
> > > > reames wrote:
> > > > > I think you mean: must be exactly N bytes.  If not, I'm confused.
> > > > Yeah, I could've worded this better. I was trying to say "block of bytes that the `MemoryLocation` references" (e.g. a `store i32 1, i32* %a` implies that `%a` must be *at least* 4 bytes large). Will update with the rebase.
> > > Looks like this update got lost?  
> > > 
> > > JFYI, I consider clarity on this point essential. 
> > Good catch; sorry for dropping this. Please let me know if you'd like me to expand further.
> Reading through this again, I think we have a semantic problem here.  The previous semantics of MemoryLocation was that we had a memory region which was guaranteed to be at least as large as the memory actually dereferenced.  Your documented new semantics now indicate that the accessed region is *larger* than the MemLoc when that MemLoc is precise.  This now means that to know whether a MemLoc is inclusive of the access we *must* check the precise bit.
> 
> I am not okay with that change.
> 
> I would be okay with either of the following:
> 1) Precise means "accesses exactly N bytes".  (i.e. still bounded from above, but now also bounded from below with the same value)
> 2) We replace "precise" with a MinSize field.  A memory location would then guarantee that the actual access was bounded from below by MinSize and above by Size.
> 
> I recommend (1) for the moment since I think that's what you've actually implemented.  (2) would be more generic, but would be basically starting over.
Option one is what I was going for initially, but I see I tripped over myself a bit (mostly, the last sentence was meant to apply only to imprecise memory locations, though I made that wholly unclear :) ). Reworded.

As for #2, yeah, we can do that if there's a great need for it, but I'm happy to start with something simpler. 


https://reviews.llvm.org/D44748