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

Fri Aug 12 08:10:06 PDT 2022

jdoerfert added a comment.

In D131628#3719136 <https://reviews.llvm.org/D131628#3719136>, @nlopes wrote:

> 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.

Let's just resolve this now and here.

> 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. [...]

I figured us using it implies we need it. However, here is some data: https://tianshilei.me/wp-content/uploads/2022/06/ipdps-2022.pdf 
Note section IV-B2 which talks about inter-procedural reachability, that is only possible with nocallback on intrinsics (and leaf on malloc and free in some cases).
GridMini is a really simple test case, anything with more complexity (which causes the kernel to be not completely inlined) benefits explicitly from this.

> 2. [...]  A simple hack would be to drop all nocallback attributes when linking IR files. That would fix the LTO case.

`nocallback` follows, as I mentioned before, the same idea as `inaccesiblememory`. You are not wrong about LTO, but we can apply the same handling to both which is to drop the attribute during a llvm-link step. We only need to do that if we link in the definition. That is why I said this should only be valid on declarations. (inaccessiblememory should also have that restriction).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131628