murdock: also compile with LLVM/clang by miri64 · Pull Request #9398 · RIOT-OS/RIOT

miri64 · 2018-06-22T08:36:15Z

Contribution description

This is just an RFC and just a sketch at the moment, as I don't know how good this idea is (may slow down Murdock builds, but so would any new board) or if what I'm doing here is even correct.

But there are two major reasons for this:

clang picks up different errors sometimes than GCC.
OSX support is hardened as it is usually the toolchain used there.

Issues/PRs references

None (but idea sparked by #8646 (comment))

miri64 · 2018-06-22T08:37:53Z

Requested review by @kaspar030 since it is his code I'm fiddling around with and by @gebart since IIRC he introduced the TOOLCHAIN feature.

miri64 · 2018-06-22T08:55:46Z

Seems to work now + Murdock already picked out some errors ;-)

miri64 · 2018-06-22T08:57:10Z

.murdock

+        echo "gnu llvm"
+    else
+        echo "gnu"
+    fi


Maybe this construct could be replaced with some BOARD=$board make info-toolchains-supported target? This way also the ARM platforms will be build with LLVM.

miri64 · 2018-06-22T09:00:19Z

Seems to work now + Murdock already picked out some errors ;-)

Except for one error in can all seem to be in pkgs...

vincent-d · 2018-06-22T17:10:49Z

Except for one error in can all seem to be in pkgs...

Funny one, this seems to come from LL_PREPEND_ELEM from utlist.h. Not a clue how to fix it though!

jnohlgard · 2018-06-23T09:39:25Z

I think the best way is just to ignore the Wtautological-pointer-compare warning for that line.
See http://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas

Use something in utlist.h like the example in the clang manual

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wtautological-pointer-compare"

Blah

#pragma clang diagnostic pop

cladmi · 2018-07-03T10:22:16Z

I would really like to have llvm for native and llvm for at least few reference boards.
Just doing one board per architecture could already help maintaining the build system.

miri64 · 2018-07-03T12:12:26Z

@cladmi any idea how to achieve this based on the changes I already provided (keeping #9398 (comment) in mind)?

cladmi · 2018-07-06T17:26:46Z

@miri64 I did not really review for the moment.

cladmi · 2018-07-09T12:37:37Z

@miri64 I would say it depends, on what is wanted, test all possible boards or just some.

If it is focusing first only on CI where some boards are selected for testing, adding other names at the place where you put native and check if the board is in the "test with llvm" list. Using https://stackoverflow.com/a/24495744/395687 oneliner for example.
Good thing, it starts small and does not double the number of jobs.

If it is meant to be global, the info-toolchains-supported or maybe only info-toolchains-supported-on-ci could be an idea too as the configuration could be put in cpu common directories. But for the global info-toolchains-supported, as a user I think I would expect to check the computer configuration, like is llvm installed, does it support this architecture, which is a feature on its own and maybe not only read a static configuration that could fail for me.

miri64 · 2018-07-09T12:51:26Z

Just doing one board per architecture could already help maintaining the build system.

If it is focusing first only on CI where some boards are selected for testing, adding other names at the place where you put native and check if the board is in the "test with llvm" list. Using https://stackoverflow.com/a/24495744/395687 oneliner for example.
Good thing, it starts small and does not double the number of jobs.

[…] which is a feature on its own and maybe not only read a static configuration that could fail for me.

Ok, then considering these three statements and with the urge to keep it simple for now, I would then check platform is native or a selection of a few other ARM-based boards in the hard-coded manner I applied for now and we later on select to provide a make target.

miri64 · 2018-07-09T12:52:21Z

or a selection of a few other ARM-based boards

(to my knowledge non of our other architectures have llvm-support at the moment)

miri64 · 2018-07-09T13:04:44Z

I added samr21-xpro and one random nucleo board with Cortex-M4 now. Most other boards I tried did not compile :(

cladmi · 2018-07-09T13:08:07Z

It looks like murdock cannot generated the aggregated report with this added parameter.

From guessing and if I understand correctly I would imagine that adding -E TOOLCHAIN to DWQ_ENV may help there.
If this is used to separate jobs, I think it would prevent two different build to generate an output. @kaspar030 would know.

miri64 · 2018-07-09T13:09:59Z

Let's try that ;-)

miri64 · 2018-07-09T13:27:39Z

From guessing and if I understand correctly I would imagine that adding -E TOOLCHAIN to DWQ_ENV may help there.

Nope, with that it doesn't show any build results anymore ;-)

cladmi · 2018-07-17T13:48:20Z

.murdock

+    local board=$1
+
+    # TODO provide make target in similar fashion as the info-boards-supported one
+    for b in native samr21-xpro nucleo-f401re


I would prefer to have this list in a global variable outside of the function to not have this "magic" list in the function.

+1, similar to TEST_BOARDS_AVAILABLE

cladmi · 2018-07-17T13:48:46Z

@kaspar030 do you have any idea how to fix the output ?

kaspar030 · 2018-07-17T19:27:06Z

.murdock

+    local board=$1
+
+    # TODO provide make target in similar fashion as the info-boards-supported one
+    for b in native samr21-xpro nucleo-f401re


please re-use the is_in_list function.

kaspar030 · 2018-07-17T19:32:17Z

do you have any idea how to fix the output ?

I'll have a look.

miri64 · 2018-08-16T19:04:07Z

Rebased

miri64 · 2018-08-17T14:18:53Z

I talked about this PR with @cladmi over lunch and we decided to remove the failing architectures from the build list for add them in follow-ups, so we can finally proceed with this PR.

miri64 · 2018-08-17T14:23:39Z

Done

miri64 · 2018-08-17T14:27:31Z

Note to self: old state of .murdock is in dfc7c68

miri64 · 2018-08-17T20:00:47Z

@kaspar030 can you have another look?

miri64 · 2018-08-17T20:43:35Z

.murdock

 RUN_TESTS=${RUN_TESTS:-${NIGHTLY}}

-DWQ_ENV="-E BOARDS -E APPS -E NIGHTLY -E RUN_TESTS"
+DWQ_ENV="-E BOARDS -E APPS -E NIGHTLY -E RUN_TESTS -E TOOLCHAIN"


@cladmi and I weren't really sure if this is required

From what I understand now, it should not be necessary.

The compile tasks including toolchain will be listed by

RIOT/.murdock

Lines 125 to 127 in a8c53ec

dwqc ${DWQ_ENV} -s \

"$0 get_app_board_toolchain_pairs \${1}" \

| xargs '-d\n' -n 1 echo $0 compile

And set from the argument in

RIOT/.murdock

Lines 136 to 139 in a8c53ec

compile() {

local appdir=$1

local board=$(echo $2 | cut -f 1 -d':')

local toolchain=$(echo $2 | cut -f 2 -d':')

I removed this change, so let's see how murdock might react.

Seems to work regardless :-)

cladmi

Good for me. You can squash.

There are two major reasons for this: 1. clang picks up different errors sometimes than GCC. 2. OSX support is hardened as it is usually the toolchain used there.

miri64 · 2018-08-20T14:03:00Z

Squashed

cladmi · 2018-08-20T15:20:37Z

@kaspar030 do you have any remarks left before merging ?

kaspar030 · 2018-08-20T18:02:48Z

I'm happy here. Good job guys!

miri64 requested review from jnohlgard and kaspar030 June 22, 2018 08:36

miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 22, 2018

miri64 force-pushed the murdock/enh/llvm branch from 3e8b7ba to 140456b Compare June 22, 2018 08:48

miri64 commented Jun 22, 2018

View reviewed changes

cladmi added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 9, 2018

cladmi reviewed Jul 17, 2018

View reviewed changes

kaspar030 requested changes Jul 17, 2018

View reviewed changes

cladmi added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 16, 2018

miri64 force-pushed the murdock/enh/llvm branch from 237dfbe to 1394027 Compare August 16, 2018 19:04

miri64 mentioned this pull request Aug 16, 2018

pkg: blacklist selected pkgs for LLVM/clang #9734

Merged

cladmi mentioned this pull request Aug 17, 2018

Makefile.include: configure '--output-sync=target' #9792

Closed

miri64 force-pushed the murdock/enh/llvm branch from 0754011 to 7edeabe Compare August 17, 2018 14:23

miri64 force-pushed the murdock/enh/llvm branch 2 times, most recently from d739fa8 to 42c5b00 Compare August 17, 2018 14:25

miri64 commented Aug 17, 2018

View reviewed changes

cladmi approved these changes Aug 20, 2018

View reviewed changes

murdock: also compile with LLVM/clang

9160b9c

There are two major reasons for this: 1. clang picks up different errors sometimes than GCC. 2. OSX support is hardened as it is usually the toolchain used there.

miri64 force-pushed the murdock/enh/llvm branch from 83013d9 to 9160b9c Compare August 20, 2018 14:03

kaspar030 approved these changes Aug 20, 2018

View reviewed changes

kaspar030 merged commit 5710fd8 into RIOT-OS:master Aug 20, 2018

miri64 deleted the murdock/enh/llvm branch August 20, 2018 20:53

miri64 mentioned this pull request Aug 21, 2018

murdock: add selected cortex-m boards to LLVM build #9809

Merged

This was referenced Sep 21, 2018

"Minor" compiling issues found by clang #7875

Closed

tests/gnrc_sixlowpan: fix stacksize of dummy device #10024

Merged

	dwqc ${DWQ_ENV} -s \
	"$0 get_app_board_toolchain_pairs \${1}" \
	\| xargs '-d\n' -n 1 echo $0 compile

	compile() {
	local appdir=$1
	local board=$(echo $2 \| cut -f 1 -d':')
	local toolchain=$(echo $2 \| cut -f 2 -d':')

Conversation

miri64 commented Jun 22, 2018

Contribution description

Issues/PRs references

Uh oh!

miri64 commented Jun 22, 2018

Uh oh!

miri64 commented Jun 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 commented Jun 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincent-d commented Jun 22, 2018

Uh oh!

jnohlgard commented Jun 23, 2018

Uh oh!

cladmi commented Jul 3, 2018

Uh oh!

miri64 commented Jul 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cladmi commented Jul 6, 2018

Uh oh!

cladmi commented Jul 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miri64 commented Jul 9, 2018

Uh oh!

miri64 commented Jul 9, 2018

Uh oh!

miri64 commented Jul 9, 2018

Uh oh!

cladmi commented Jul 9, 2018

Uh oh!

miri64 commented Jul 9, 2018

Uh oh!

miri64 commented Jul 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cladmi commented Jul 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaspar030 commented Jul 17, 2018

Uh oh!

miri64 commented Aug 16, 2018

Uh oh!

miri64 commented Aug 17, 2018

Uh oh!

miri64 commented Aug 17, 2018

Uh oh!

miri64 commented Aug 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miri64 commented Aug 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

miri64 commented Aug 20, 2018

Uh oh!

cladmi commented Aug 20, 2018

Uh oh!

miri64 commented Jun 22, 2018 •

edited

Loading

miri64 commented Jul 3, 2018 •

edited

Loading

cladmi commented Jul 9, 2018 •

edited

Loading

miri64 commented Aug 17, 2018 •

edited

Loading