[PATCH] D155894: [BPF] Do not miscompile, allow external calls

Mon Jul 24 12:21:09 PDT 2023

eddyz87 added a comment.

In D155894#4528546 <https://reviews.llvm.org/D155894#4528546>, @tamird wrote:

>> - change of the behavior for `ExternalSymbolSDNode`: this was not a part of Diff 1, so I assume that it was not a part of the @tamird's initial intent. Is there a use case for this change? The libbpf commit 0a34245 <https://github.com/torvalds/linux/commit/0a34245> deals only with external maps, not external functions.
>
> When compiling Rust code, we see this warning trigger on `memset` intrinsics, but this is not a problem because we provide an implementation of `memset` and the intrinsic is eventually lowered to a call to the function we provide. Is there a finer-grained change that would achieve our goals?

Could you please point me to some example (on github or something like that)?
I want to insert some prints to see what is the difference in DAG nodes between this and functions declared `extern` in C code.

>> - changes in `LowerCallResult` and `LowerOperation` regarding undef: I think both zeros and undef's are a bad choice here. If we can't compile it we should report error and abort compilation.
>
> I agree in principle, but as note in code comments, this produces worse diagnostics when multiple functions contain such errors. Consider also that it was important to avoid exiting, hence D20571 <https://reviews.llvm.org/D20571>, but I do not understand why. Perhaps @ast can fill this in.

If we want to report this for multiple functions I think we can set some flag and exit in `TargetLoweringBase::finalizeLowering()`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155894