On 23 May 2017 at 07:33, Bill Fischofer <bill.fischo...@linaro.org> wrote: > On Mon, May 22, 2017 at 11:12 PM, Brian Brooks <brian.bro...@arm.com> wrote: > >> On 05/22 22:16:27, Bill Fischofer wrote: >> > Part 6 still has checkpatch issues: >> > >> > WARNING: Missing a blank line after declarations >> > #464: FILE: platform/linux-generic/include/odp_schedule_scalable.h:64: >> > + uint16_t qschst_type; >> > + ringidx_t prod_read SPLIT_PC; >> > >> > WARNING: Missing a blank line after declarations >> > #468: FILE: platform/linux-generic/include/odp_schedule_scalable.h:68: >> > + odp_buffer_hdr_t **prod_ring; >> > + ringidx_t cons_write SPLIT_PC; >> >> These two are not real issues. SPLIT_PC will be replaced by the >> preprocessor, >> which confuses checkpatch. >> >> > WARNING: 'noticably' may be misspelled - perhaps 'noticeably'? >> > #577: FILE: >> > platform/linux-generic/include/odp_schedule_scalable_config.h:34: >> > + * On x86, this decreases overhead noticably. >> >> Had to symlink codespell's dictionary.txt to /usr/share/codespell/ >> dictionary.txt >> since it is in a different path on my system. Fixed in v7. >> >> > This still fails to compile using clang on 32-bit x86 systems: >> >> We've already discussed that this is a compiler versioning issue since it >> compiles fine with clang 4.0.0. >> > > There is currently no LTS version of Ubuntu that includes clang 4.0.0, so > I'd say we need to fix this. In any event the fix seems trivial so why not > do it? >
I do not think we need to tie the distro version and the compiler version. In another linux distro, it might be possible to install latest clang. It also looks to me like it is a compiler issue, as the latest version of the compiler is not throwing the error. Clang 4.0.0 seems to be a major release (going by the version number), I think we should move to this version. > >> >> How do we want to handle issues like these since it is not practical to >> require contributors to build with every range of compiler (or on multiple >> distros) claimed to be supported by the project? I think we need CI here. >> >> > CC odp_schedule_scalable.lo >> > odp_schedule_scalable.c:1630:13: error: implicit conversion from >> > 'unsigned long long' to 'sched_group_mask_t' (aka 'unsigned int') >> > changes >> > value from 18446744073709551615 to 4294967295 >> > [-Werror,-Wconstant-conversion] >> > sg_free = ~0ULL; >> > ~ ^~~~~ >> > 1 error generated. >> > Makefile:1019: recipe for target 'odp_schedule_scalable.lo' failed >> > make[1]: *** [odp_schedule_scalable.lo] Error 1 >> > >> > It's sufficient to say: >> > >> > sg_free = ~0; >> > >> > and on the else arm: >> > >> > sg_free = (1 << bits) - 1; >> > >> > I've verified that this syntax is acceptable to both gcc and clang in >> both >> > 32 and 64-bit modes. >>
