[PATCH] D131628: [LangRef] Add description for nocallback attribute

Fri Aug 12 07:38:09 PDT 2022

nlopes requested changes to this revision.
nlopes added a comment.
This revision now requires changes to proceed.

In D131628#3716252 <https://reviews.llvm.org/D131628#3716252>, @jdoerfert wrote:

> The idea is the same. So something like:
>
>   This attribute indicates that the function may only execute code that is not part of the module being compiled before return from the function. 

But this wording is not useful to lower gcc/clang's leaf attribute. The semantics don't match then. The semantics on LTO is different.
The gcc attribute only guarantees that a leaf fn call doesn't change any global in the present file. Doesn't guarantee that it won't change a global in another file. The proposed wording doesn't match this semantics.

I only see 2 ways forward:

1. Drop the lowering from clang's leaf attribute. We've no data to support that there's any performance benefit
2. Someone provides performance data justifying the effort and we take time to properly design this. A simple hack would be to drop all nocallback attributes when linking IR files. That would fix the LTO case.

>> What's the usefulness of this attribute? Is there any optimization that is enabled by this? It's not obvious to me, so it would be nice to document that.
>
> We use it in the Attributor to build a better "call graph". "Default intrinsics" carry it, see https://reviews.llvm.org/D118680. This helps us to do proper reachability queries when we optimize memory accesses interprocedurally but have some intrinsic calls flying around. We also annotate external functions in the OpenMP GPU runtime for the same reason, see https://reviews.llvm.org/D112153.

Thanks.
I see the point now. You can decide that a variable whose address has escaped won't be changed by the fn call. Unclear what's the perf benefit without data though.

Anyway, the semantics issue pointed out above must be resolved or else the patch must be reverted as currently the lowering of clang is wrong and will miscompile programs.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131628