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

[Eduard Zingerman via Phabricator via llvm-commits]

eddyz87 added a comment.

Please note that a number of unit tests fails with this patch applied:

- LLVM :: CodeGen/BPF/many_args1.ll
- LLVM :: CodeGen/BPF/many_args2.ll
- LLVM :: CodeGen/BPF/struct_ret1.ll
- LLVM :: CodeGen/BPF/vararg1.ll
- LLVM :: CodeGen/BPF/warn-call.ll

These failures could be caught when `ninja check-all` target is executed.

I don't think that cleanup changes should be intermixed with functionality changes.
As the summary notes there three separate changes in this revision:

- code cleanup (which is, in fact, somewhat controversial):
  - LLVM Coding Standards actually require brace-less if statements (link <https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements>)
  - I don't understand the crusade against `assert` and `llvm_unreachable`
- 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.
- 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 think that all three should be tracked as separate revisions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155894