[PATCH] D61652: [LangRef][Attr] Clarify dereferenceable(_in_scope)

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert marked an inline comment as done.
jdoerfert added a subscriber: reames.
jdoerfert added inline comments.


================
Comment at: llvm/docs/LangRef.rst:1164
+
+``dereferenceable_in_scope(<n>)``
+    This indicates that the annotated pointer value has the
----------------
sanjoy wrote:
> I'm not sure if this is a good semantic property.  Consider (the CFG form of):
> 
> ```
> if (cond) {
>   a = dereferenceable_in_scope(4) ...
> } else {
>   ...
> }
> side_effect_that_frees_a()
> ```
> 
> I think this program above is well defined because the definition of `a` does not dominate `side_effect_that_frees_a()`.  But that means we can't tail duplicate the program into:
> 
> ```
> if (cond) {
>   a = dereferenceable_in_scope(4) ...
>   side_effect_that_frees_a()
> } else {
>   ...
>   side_effect_that_frees_a()
> }
> ```
> 
> Same applies for inlining.  Any transform that can extend the dominance region of some value needs to be audited.
> 
> What is the use case for this attribute?  Can we do something more targeted for this use case?
> What is the use case for this attribute? Can we do something more targeted for this use case?

My "use case" is basically the clang interpretation of (current) `derferenceable` which I tried to replicate with the new `derferenceable` definition. There are users of (current) `derferenceable` that interpret it as `derferenceable_in_scope` and they should have a way to do so.

> I think this program above is well defined because the definition of a does not dominate 
> Same applies for inlining. Any transform that can extend the dominance region of some value needs to be audited.

I agree that there is a problem with the current wording wrt. inlining. I'll try to fix that.
Maybe @reames can help me define what they want/need.

For the record, the example above is not sufficient to expose the inlining problem though. Above, a was "accessible" at the location where a was freed, so the `dereferenceable_in_scope` was violated already. Thinking about it, even if we inline, SSA dominance property should be combined with reachability to a user to give you what you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61652