> Am 09.07.2019 um 20:14 schrieb Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>: > > On Mon, Jul 8, 2019 at 8:01 AM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote: >> >>> Am 05.07.2019 um 16:22 schrieb Daniel Borkmann <daniel@xxxxxxxxxxxxx>: >>> >>> On 07/01/2019 08:40 PM, Ilya Leoshkevich wrote: >>>> Am 01.07.2019 um 17:31 schrieb Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>: >>>>> Do we still need clang | llc pipeline with new clang? Could the same >>>>> be achieved with single clang invocation? That would solve the problem >>>>> of not detecting pipeline failures. >>>> >>>> I’ve experimented with this a little, and found that new clang: >>>> >>>> - Does not understand -march, but -target is sufficient. >>>> - Understands -mcpu. >>>> - Understands -Xclang -target-feature -Xclang +foo as a replacement for >>>> -mattr=foo. >>>> >>>> However, there are two issues with that: >>>> >>>> - Don’t older clangs need to be supported? For example, right now alu32 >>>> progs are built conditionally. >>> >>> We usually require latest clang to be able to test most recent features like >>> BTF such that it helps to catch potential bugs in either of the projects >>> before release. >>> >>>> - It does not seem to be possible to build test_xdp.o without -target >>>> bpf. >>> >>> For everything non-tracing, it does not make sense to invoke clang w/o >>> the -target bpf flag, see also Documentation/bpf/bpf_devel_QA.rst +573 >>> for more explanation, so this needs to be present for building test_xdp.o. >> >> I'm referring to the test introduced in [1]. test_xdp.o might not be an >> ideal target, but even if it's replaced with a more suitable one, the >> llc invocation would still be required. So I could redo the patch as >> follows: >> >> - Replace test_xdp.o with get_cgroup_id_kern.o, use an intermediate .bc >> file. >> - Use clang without llc for all other eBPF programs. >> - Split out Kbuild include and order-only prerequisites. >> >> What do you think? > > How about just forcing llc to fail as well like this: > > (clang <whatever> || echo "clain failed") | llc <whatever> > > While not pretty, it will get us what we need with very clear > messaging as well. E.g.: > > progs/test_btf_newkv.c:21:37: error: expected identifier > PF_ANNOTATE_KV_PAIR(btf_map_legacy, int, struct ipv_counts); > ^ > progs/test_btf_newkv.c:21:1: warning: type specifier missing, defaults > to 'int' [-Wimplicit-int] > PF_ANNOTATE_KV_PAIR(btf_map_legacy, int, struct ipv_counts); > ^ > 1 warning and 1 error generated. > llc: error: llc: <stdin>:1:1: error: expected top-level entity > clang failed > ^ While this would definitely work at least in my scenario, what about the following hypothetical cases? - clang manages to output something before exiting with nonzero rc - future llc version exits with zero rc when given "clang failed" or any other arbitrary string as an input (perhaps, with just a warning?) Come to think of it, what are the downsides of having intermediate bitcode files? While I did not run into this yet, I could imagine it might be even useful from time to time to inspect them.
