[PATCH] D155894: [BPF] allow external calls

[Tamir Duberstein via Phabricator via llvm-commits]

tamird added inline comments.


================
Comment at: llvm/lib/Target/BPF/BPFISelLowering.cpp:466
     Callee = DAG.getTargetExternalSymbol(E->getSymbol(), PtrVT, 0);
-    fail(CLI.DL, DAG, Twine("A call to built-in function '"
-                            + StringRef(E->getSymbol())
----------------
ast wrote:
> How does it help aya?
> 
> In general I think it's ok to remove this, but we need to define the behavior more accurately here.
> Does it mean that we promote builtin to global ? What is the visibility of such symbol? What relocation do we use ? What libbpf and libbpf linker should do with it?
> 
> In particular with memcpy, do we generate asm that calls "llvm.memcpy.p0.p0.i64" name or does it become plain "memcpy" ?
> How does it help aya?

In aya, we provide implementations of these built-ins (memset, memcpy, etc.). In bpf-linker, we combine all the IR into a single module before codegen. Currently this error fires, which we must treat as fatal or ignore by string matching.

It turns out that codegen generates a call to plain "memset", and the resulting code is completely valid. So this error is spurious.

> In general I think it's ok to remove this, but we need to define the behavior more accurately here.
> Does it mean that we promote builtin to global ? What is the visibility of such symbol? What relocation do we use ? What libbpf and libbpf linker should do with it?

Looks like yonghong discovered that this symbol is indeed promoted to global.

> 
> In particular with memcpy, do we generate asm that calls "llvm.memcpy.p0.p0.i64" name or does it become plain "memcpy" ?

Yes, it becomes "plain".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155894