[PATCH] D63243: [WIP] Adjust the users of dereferenceable wrt. dereferenceable_globally

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/include/llvm/IR/Value.h:583
+  uint64_t
+  getPointerDereferenceableBytes(const DataLayout &DL, bool &CanBeNull,
+                                 const Instruction *CtxI = nullptr) const;
----------------
jdoerfert wrote:
> reames wrote:
> > jdoerfert wrote:
> > > reames wrote:
> > > > An API suggestion.  Instead of adding a context pointer here, add a boolean OnlyAtDef out param.  Then introduce a helper function in ValueTracking which does the inference over the IR.  (i.e. leave this as returning basic facts about the value, and move the analysis into an analysis.) 
> > > > 
> > > > Or if you want, call the param AssumeNoFree, or whatever..
> > > I like it, done. Though, I do need the location of the definition, so I made it an `Instruction *&` not a `bool &`, what do you think?
> > > 
> > > 
> > I'd prefer the boolean given there's only two options: 1) on def, and 2) globally.  I'm willing to defer to you on this if you feel strongly though.  
> My problem is that for option 1) "on def", the information that is returned would not be sufficient for most users. In the "on def" case you also need the "definition" to query `maybeFreedInBetween`. If I would return a boolean, I would need to implement a second traversal that does the same just to find the definition. Does this make sense?
No, not really.  You can only ask this question on a particularly value.  (i.e. argument or return value or load)  That is the definition.  If you have the instruction to query the property, you have the def, so I really don't understand your point.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63243