[clang] 24ab9b5 - Generalize the pass registration mechanism used by Polly to any third-party tool

[Serge Guelton via cfe-commits]

On Mon, Jan 06, 2020 at 03:45:15PM -0800, Eric Christopher wrote:
> Hi Serge,

Hi Eric,

> I have a few questions here about this:
> 
> In general this appears to be a lot more complex than the existing plugin
> solutions and requires quite a lot of custom cmake rules that are difficult to
> maintain. Why do we want this in general for the project? I understand the
> desire to make polly less of a special case, but I don't think that the overall
> complexity is worth the added ability to register polly plugins. Perhaps
> disabling the polly plugin registration scheme is a better option?

Indeed, the cmake machinery is now more generic (making Polly an application of
it, instead of an exception), at the expense of being more complex.

Why would we want that? When trying to write a custom pass, the usual process,
is to write a plugin that works correctly with opt, and that step is relatively
easy. To achieve a decent clang integration, it gets more difficult, and if one
still take the external plugin approach, one ends up with very long and non
standard command line like -Xclang -load -Xclang MyPlugin.so etc. So one need
another step to integrate the plugin into the static build, which means forking
llvm-project, rebasing every now and then etc.

The proposed infrastructure makes this process smoother and non intrusive to the
llvm-project codebase: all development can be done in a separate git repo,
integration is controlled through cmake flags, and integration to
clang/opt/bugpoint is built-in.

> That being said, perhaps it is worth it? But I think we need to call out why we
> want it. I would also have expected something to llvm-dev for a change of this
> magnitude. I didn't see anyone from the pass manager hierarchy on the reviews
> and the final reviewer wasn't someone who contributes to these areas typically.

I've (obviously) mentioned this development on llvm-dev, see

    http://lists.llvm.org/pipermail/llvm-dev/2019-September/135326.html

Is there anything I should have done? Probably reaching llvm-dev before
commiting. Reaching the right reviewers has always been a challenge to me, I had
hoped that the mail to llvm-dev would trigger some subscription :-)

> In addition, what's with the OSX failure? It's currently turned off and was
> breaking the bots, but does it mean that you don't expect this machinery to
> work on OSX? That seems like a severely limiting factor for the project.

I've setup github actions to test many configurations before merging [0], but missed one of
them. I'm currently working on fixing that part.

> +One first needs to create an independent project and add it to either ``tools/
> ``
> +or, using the MonoRepo layout, at the root of the repo alongside other
> projects.
> 
> This appears to be from an earlier incarnation of the patch? There's only one
> layout now.

Correct! I'll fix that.

> 
>  ; CHECK-EP-VECTORIZER-START-NEXT: Running pass: NoOpFunctionPass
> +; CHECK-EXT: Running pass: {{.*}}::Bye on foo
> 
> Why is this running on every test of the pass manager? It should be an example
> run in the examples directory and not on by default? Same for every other PM
> test. This seems like a bug?

It's not. When the examples are active and if the appropriate cmake flag is set
(which is not the case by default), the pass is linked in statically, and is run
in the default pipeline. The CHECK-EXT prefix is disabled otherwise. That's one
of the configuration I did test :-)

> Thanks!

Thanks for giving me the opportunity to clarify some obscure points, hope it
helps !


[0] https://github.com/serge-sans-paille/llvm-project/commit/37c9b5080acd3d7543347de109e44c402626d504/checks?check_suite_id=346849887