Page MenuHomePhabricator

Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.
ClosedPublic

Authored by chandlerc on Dec 22 2016, 1:09 AM.

Details

Summary

Much to my surprise, '-disable-llvm-optzns' which I thought was the
magical flag I wanted to get at the raw LLVM IR coming out of Clang
deosn't do that. It still runs some passes over the IR. I don't want
that, I really want the *raw* IR coming out of Clang and I strongly
suspect everyone else using it is in the same camp.

I've not talked to anyone debugging Clang and LLVM interactions that
needed these different flags so I'd like to remove the less widely known
one (I think) and consolidate on a simple, strong model for the one flag
left.

This is part of simplifying how Clang drives LLVM to make it cleaner to
wire up to the new pass manager.

Diff Detail

Repository
rL LLVM

Event Timeline

chandlerc updated this revision to Diff 82313.Dec 22 2016, 1:09 AM
chandlerc retitled this revision from to Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing..
chandlerc updated this object.
chandlerc added reviewers: rsmith, mehdi_amini.
chandlerc added a subscriber: cfe-commits.
joerg added a subscriber: joerg.Dec 22 2016, 3:35 AM

There are two issues I have with the flags right now. The first one you are addressing with this patch. The second issue is they remain CC1-only flags. Can we introduce a proper "-emit-raw-llvm" flag to work like "-emit-llvm" but without the additional IR processing. We can make "-disable-llvm-optzns" an alias on the CC1-level for backwards compatibility. It is fine as a second step, but it would be nice to consider the big picture before shuffling things around.

rnk added a subscriber: rnk.Dec 22 2016, 8:52 AM

How about standardizing on -disable-llvm-passes instead of -disable-llvm-optzns? I never liked "optzns" and can't remember how to spell it. Also, this flag disables LLVM instrumentation passes (ASan) as well as optimization passes, so I think -disable-llvm-passes is the better name.

mehdi_amini edited edge metadata.Dec 22 2016, 9:45 AM
In D28047#629746, @rnk wrote:

How about standardizing on -disable-llvm-passes instead of -disable-llvm-optzns? I never liked "optzns" and can't remember how to spell it. Also, this flag disables LLVM instrumentation passes (ASan) as well as optimization passes, so I think -disable-llvm-passes is the better name.

I agree with that.
To be friendly with developers/users we could print an error mentioning to use -disable-llvm-passes when they try to use -disable-llvm-optzns (which we accept as a -mllvm -disable-llvm-optzns hardcoded in the driver by the way).

In D28047#629746, @rnk wrote:

How about standardizing on -disable-llvm-passes instead of -disable-llvm-optzns? I never liked "optzns" and can't remember how to spell it. Also, this flag disables LLVM instrumentation passes (ASan) as well as optimization passes, so I think -disable-llvm-passes is the better name.

I agree with that.
To be friendly with developers/users we could print an error mentioning to use -disable-llvm-passes when they try to use -disable-llvm-optzns (which we accept as a -mllvm -disable-llvm-optzns hardcoded in the driver by the way).

I only have a mild preference for how we spell the name based on the fact that i've told people to use -disable-llvm-optzns for years. If we want to go with ...-passes, fine with me.

I'll probably just add an alias so that you can use either spelling, but change the tests to consolidate on whichever spelling we end up with.

SG? I can regenerate the patch if desired, but it'll just be a lot of perl...

LGTM, but please wait for @rnk to confirm :)

Also any opinion about a driver option -emit-raw-llvm as @joerg suggested?

rnk added a comment.Dec 22 2016, 11:28 AM

I only have a mild preference for how we spell the name based on the fact that i've told people to use -disable-llvm-optzns for years. If we want to go with ...-passes, fine with me.

We have some freedom to change the -cc1 interface. I liked being able to pass -g to -cc1, but then we went to -debug-info-kind=(limited|full|linetables) or whatever. =?

I'll probably just add an alias so that you can use either spelling, but change the tests to consolidate on whichever spelling we end up with.

SG? I can regenerate the patch if desired, but it'll just be a lot of perl...

Sounds good! We definitely don't need both -disable-llvm-optzns and -disable-llvm-passes.

LGTM, but please wait for @rnk to confirm :)

Also any opinion about a driver option -emit-raw-llvm as @joerg suggested?

I like the idea generally, but I don't have an immediate use case and would defer that to a subsequent patch authored by someone who wants this? I really only wanted to remove the divergence between two flags here.

mehdi_amini accepted this revision.Dec 22 2016, 2:48 PM
mehdi_amini edited edge metadata.

LGTM, but please wait for @rnk to confirm :)
Also any opinion about a driver option -emit-raw-llvm as @joerg suggested?

I like the idea generally, but I don't have an immediate use case and would defer that to a subsequent patch authored by someone who wants this? I really only wanted to remove the divergence between two flags here.

Sure, even @joerg mentioned it was fine to leave for later in his suggestion, I just didn't want it to go unnoticed.

This revision is now accepted and ready to land.Dec 22 2016, 2:48 PM
This revision was automatically updated to reflect the committed changes.
vasich added a subscriber: vasich.Dec 4 2017, 2:10 PM