[llvm-dev] RFC: Building GlobalISel by default

[Mehdi Amini via llvm-dev]

> On Jan 14, 2017, at 4:57 AM, Renato Golin via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> On 14 January 2017 at 01:54, Quentin Colombet via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> Now, four backends (if I am counting right: X86, ARM, AArch64, AMDGPU) are working on bringing-up GlobalISel, I’d like to switch the default of the LLVM_BUILD_GLOBAL_ISEL variable in CMake, such that the framework gets built by default.
> 
> Hi Quentin,
> 
> I'm not extremely confident in a full switch right now, mainly due to
> Michael's concerns. The ARM port is in its initial stage and there
> could still be many refactorings and destructive changes. I'm not
> foreseeing a cross between the isel tests, but every failure makes the
> bots red, and if Global ISel incurs in more failures due to its stage
> in the development cycle, we'll start seeing the bots more red than
> we'd like. Right now, it's already very likely that you'll receive bot
> emails on commits, I don't want to increase that.

I don’t see why having global ISel is supposed to increase the rate. `make check` will runs on the committer machine with GlobalISel enabled.

The good question from Michael is IMO "if my (non-GlobalISel) change breaks GlobalISel, how likely is it to be a bug in my code vs. a bug in GlobalISel?”

But that’s totally unrelated to the bots, since it’ll fails on the committer machine as well.

> 
> One alternative is to create a buildbot that turns it on by default,
> build all back-ends that use GlobalISel and then let it running for a
> while. I don't think we're self-hosting on any architecture right now,
> nor we're passing the test-suites, so the bot will probably just be a
> simple "make check-all", which is more than enough for the time being.

This is in place for months: http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-globalisel/


> 
> As soon as we can self-host and pass the test-suite, I think we can
> enable it by default.

I don’t understand why it is an interesting property to be able to pass the test-suite to have `make check` testing it.

> 
> Some comments inline...
> 
> 
>> * Upsides *
>> 
>> For people developing on GlobalISel, it will:
>> - Simplify the CMake command to type :)
>> - Build/Test GlobalISel on all the LLVM bots
> 
> This is already covered by the people developing Global ISel on the
> different architectures. As an experimental feature, I'm ok with the
> round time of "ARM developers picking up x86 code breaking their
> stuff".
> 
> 
>> For people not developing on GlobalISel, it will:
>> - Test that GlobalISel still works with your changes (make check will test that for you)
> 
> A separate buildbot will do that for you.
> 
> 
>> - Allow you to play with it!
> 
> The cost of adding one CMake option is really small.
> 
> 
>> * Downsides *
>> 
>> For people developing on GlobalISel, it will:
>> - Leave the status as it is now, meaning that mainly only people working on GlobalISel look at the failures of GlobalISel specific bots
> 
> If we already have those, I don't see why we need more. We're not
> self-hosting not passing the test-suite. the "check-all" tests should
> not matter which platform they run.

The `check-all` are *not* testing GlobalISel right now because GlobalISel is not *built* on the bots. This is *exactly* the point of the proposal: have `make` building the global isel code on every platform, and have `make check` testing the lit-tests for GlobalISel on every platform.
This is *not* about enabling GlobalISel  *by default* on any target AFAIK.


— 
Mehdi


> 
> 
>> For people not developing for GlobalISel, it will:
>> - Increase the compile time since the GlobalISel framework and the related target specific parts will have to be built
>> - Increase the size of the binary (depending on what backend you pull)
>> - Require the setting of an additional CMake variable to disable it (-DLLVM_BUILD_GLOBAL_ISEL=OFF)
> 
> This has a higher impact on slow bots, such as ARM and MIPS, and it's
> not a trivial amount of time and space.
> 
> On previous occasions (for example integrated-asm, lld, vecorizer), we
> have used the "self-host + test-suite passing" model and it worked
> well. Meaning, before it passes those two, it should have special
> buildbots, after, it can be turned on by default.
> 
> To move to the new technology we need an additional step, which is to
> fix most remaining bugs, which we have done for the vectorizer and
> integrated-asm, but not lld, and that's why the two first are not only
> built by default, but enabled by default, while the latter isn't.
> 
> I want Global ISel to be a success as much as you do, but I'd rather
> go through a smooth path, even if it takes a bit longer.
> 
> cheers,
> --renato
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev