This reverse iteration buildbot builds LLVM, Clang and Polly with
LLVM_REVERSE_ITERATION enabled, and subsequently runs "make check-all". With
LLVM_REVERSE_ITERATION enabled, all supported unordered llvm containers would be
iterated in reverse order.
Details
Diff Detail
Event Timeline
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
805–806 | This is actually a clean up for the PollyBuilder. The PollyBuilder has a "make polly-check-format" step which is no longer needed since "make check-polly" (or "make check-all" which we are adding) also includes it. Since we are reusing getPollyBuildFactory in the reverse iteration buildbot, it seems to be a good time to clean up the PollyBuiler at the same time. | |
zorg/buildbot/builders/PollyBuilder.py | ||
30 ↗ | (On Diff #109256) | We want to add a "make check-all" step in the reverse iterator buildbot, but only "make check-polly" is available in getPollyBuildFactory currently. Therefore we are introducing "checkAll" to control whether to run "make check-all" or "make check-polly". "checkAll" defaults to false which is the desired behavior for existing Polly builders. For the reverse iteration buildbot, "checkAll" is set to true to run "make check-all" instead of "make check-polly". |
this lgtm from my side
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
805–806 | I am fine with this change! |
@vleschuk is right that it is unclear why you change the Polly builder. I would suggest to motivate in the commit message that you modify the Polly builder to also include Polly into these builds!
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
805–806 | That's ok. But I'd prefer more granular commits. Could you please split changes into separate commits next time? |
We could even split things in smaller commits this time, I assume. If there are concerns, I believe these should be addressed.
One could e.g.:
- Remove formatting from the polly buildbot
- Add then the new checkAll feature
- Then add the new buildbot
And please excuse the "!", this slipped there by accident.
Why do you need to change this?