[PATCH] D69596: [Attributor] Add the Attributor to the new PM pipeline

Mon Jan 27 08:02:54 PST 2020

jdoerfert added a comment.

In D69596#1783188 <https://reviews.llvm.org/D69596#1783188>, @fedor.sergeev wrote:

> > Is there any testing that does enable attributor?
>
> Okey, I did find a fair amount of tests that run individual -attributor *or* -passes=attributor (with -attributor-disable=false).
>  Are there any near plans to enable attributor (setting -attributor-disable=false by default)?

Yes. I did fix the remaining LLVM test suite bugs the other day again. I was hoping to land the SCC version (D70767 <https://reviews.llvm.org/D70767>) before I ask people to set the disabled=false flag in their local test setup and report any breakages/problems.

> I dont really see a sense in having AttributorPass in pipeline but all the sematics of it completely disabled, so running Attributor does not really runs the attributor.

It allows folks to test it locally by setting the flag. Without it being in the pipeline that is, as far as I know, really hard to do.

> If -attributor-disabled=true is the state that you expect it to exist for a prolonged period of time I would better see -attributor-disable option
>  controlling conditional inclusion of this pass into the pipeline.

We can do that but (1) I hope disabled will default to false in the next release and (2) I want to make sure we do not invalidate any pass result.

> This way when somebody sees "Running pass: AttributorPass" there will be no question on whether Attributor ran or not.

Fair point. Once we enabled it by default I would be happy with not putting it in the pipeline if the flag is set. I can do that now if you think it helps.

In D69596#1841810 <https://reviews.llvm.org/D69596#1841810>, @fedor.sergeev wrote:

> setting request-changes state to make it obvious that there are some questions here I would like to see answered.

Sorry, I wrote the SCC pass (D70767 <https://reviews.llvm.org/D70767>) and merged this into it without closing this revision.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69596/new/

https://reviews.llvm.org/D69596