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

[Daniel Thornburgh via Phabricator via llvm-commits]

mysterymath added a comment.

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

> That shows the linker already drops the nocallback attribute. However, it seems the linker only drops the attribute when linking in the definition. But I don't think this is sufficient. nocallback means you don't call any function in that file. Once you bring in more functions you can't ensure you won't call those extra functions. You'll get the call graph potentially wrong.
> The linker must always drop nocallback.
>
> I also suggest that the documentation should mention the attributed must be dropped during linking.

Here's a more concrete example:
`main.ll`:

  define i32 @main() {
  entry:
    call void @foo()
    ret i32 0
  }
  declare void @foo() nocallback

`foo.ll`:

  define void @foo() {
  entry:
    call @bar()
    ret void
  }

`bar.ll`:

  define void @bar() {
  entry:
    ret void
  }

Now, if you merge `main.ll` and `bar.ll` together, you get:

  ; ModuleID = 'merged.bc'
  source_filename = "llvm-link"
  
  define i32 @main() {
  entry:
    call void @foo()
    ret i32 0
  }
  
  ; Function Attrs: nocallback
  declare void @foo() #0
  
  define void @bar() {
  entry:
    ret void
  }
  
  attributes #0 = { nocallback }

Then, the norecurse attribute on `@foo` in the merged module is incorrect, since the implementation calls `@bar`, which is now in the same module.

Do we actually know what GCC's behavior in this case with `-fwhole-program` enabled? It's interesting that part of the description of `__attribute__((leaf))` says it's there "so files can easily be merged", but it seems like merging modules in GCC would have the same issue, unless they do provenance tracking of which functions came from which original translation unit, or if they do all decision-making that involves `__attribute__((leaf)` before the modules are merged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131628