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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert marked an inline comment as done.
jdoerfert 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;
----------------
reames wrote:
> 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.  
> 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.

I'm also confused.

Let me try to explain again what I think we should do/need and why:

We call `getPointerDereferenceableBytes` on a pointer (`llvm::Value`) to get the "known dereferenceable bytes". This method determines that by traversing the operands until a "definition" is found. If the definition is natively dereferenceable, e.g., stack or global allocations, `Instruction *&DerefKnownBefore` is `null` and there is no change to the behavior we had. If the definition is "known dereferenceable" because of attributes or metadata that says `_globally`, `Instruction *&DerefKnownBefore` is `null` and there is no change to the behavior we had. Finally, if the definition is "known dereferenceable" because of attributes or metadata that does not end in `_globally`,  `Instruction *&DerefKnownBefore` is set to the `llvm:Instruction` right after the "definition" with the attribute/metadata. This means that the number of dereferenceable bytes is known for that position but not necessarily any other position in the CFG. The caller can then use `maybeFreedInBetween` to determine if it holds for the position it is interested in. To have a tight "is potentially freed", one needs to know where it is "known dereferenceable" and where we are interested in "is dereferenceable".



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63243