[PATCH] D93189: Introduce the `!nocapture` metadata and "nocapture_use" operand bundle

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/nocapture_use.ll:42
+  %tmp4 = bitcast i32* %local to i8*
+  ret i32 %tmp2
+}
----------------
aqjune wrote:
> jdoerfert wrote:
> > aqjune wrote:
> > > (Orthogonal to this patch) would it be great to explicitly state in LangRef that it is illegal for a function call to return a nocapture pointer? I can make a super short patch for this. @nlopes was also interested in this when implementing nocapture in Alive2.
> > I'm not sure I understand.
> > 
> > FWIW, Attributor uses an internal attribute "nocapture_maybe_returned" for call arguments to improve nocapture analysis.
> I meant whether this is UB or not:
> ```
> define i8* @f(i8* nocapture %p) {
>   ret i8* %p
> }
> ```
> I think it should be UB, but this doesn't seem to be explicit in LangRef.
> 
> nocapture_maybe_returned seems great..! I believe many library calls like memcpy can have this attached to their arguments. :)
> nocapture_maybe_returned seems great..! I believe many library calls like memcpy can have this attached to their arguments.

Yep, and almost every function that has the `returned` attribute on a pointer argument. Right now we assume such calls capture which is bad. Whoever introduces the enum version should replace the string version in the Attributor.

> I meant whether this is UB or not:
> ```
> define i8* @f(i8* nocapture %p) {
>   ret i8* %p
> }
> ```
> I think it should be UB, but this doesn't seem to be explicit in LangRef.

Right. UB seems proper as there is no "value we could poison". I added an UB sentence to the new lang ref additions but not to the original nocapture attribute definition. Feel free to do so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93189