[PATCH] D155894: [BPF] allow external calls

[Alexei Starovoitov via Phabricator via llvm-commits]

ast 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())
----------------
tamird wrote:
> 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".
Since backend couldn't unroll memset it should error hard.
The linking step doesn't exist most of the time.
>From the user perspective it's better to fail at compile time instead of program loading time where the verifier errors are obscure.
I think it would be good to always unroll memset into a loop even when it's huge.
And similar with memcpy.
-fno-builtin and __attribute__((no_builtin)) tricks are unrelated to any of this.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155894