This is an archive of the discontinued LLVM Phabricator instance.

Differential D92857

[clang][cli] Don't always emit -f[no-]legacy-pass-manager
ClosedPublic

Authored by jansvoboda11 on Dec 8 2020, 9:18 AM.

Details

Reviewers
dexonsmith
Bigcheese
Commits
rG10f40576f7b4: [clang][cli] Don't always emit -f[no-]legacy-pass-manager
Summary

We don't need to always generate -f[no-]experimental-new-pass-manager.

This patch does not change the behavior of any other command line flag. (For example -triple is still being always generated.)

Diff Detail

Event Timeline

jansvoboda11 requested review of this revision.Dec 8 2020, 9:18 AM
jansvoboda11 created this revision.
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 8 2020, 9:18 AM
Herald added subscribers: llvm-commits, cfe-commits. · View Herald Transcript
Harbormaster completed remote builds in B81481: Diff 310246.Dec 8 2020, 9:19 AM
jansvoboda11 mentioned this in D92775: [clang][cli] Add flexible TableGen multiclass for boolean options.Dec 8 2020, 9:22 AM
dexonsmith added a comment.Dec 8 2020, 6:22 PM

This seems fine to me, I agree this option doesn't need to be present all the time. Thanks for clarifying that -triple is still there.

@Bigcheese, WDYT?

Bigcheese accepted this revision.Dec 9 2020, 10:23 AM

LGTM. I don't see any reason to always emit this option.

This revision is now accepted and ready to land.Dec 9 2020, 10:23 AM
dexonsmith accepted this revision.Dec 9 2020, 10:36 AM
jansvoboda11 updated this revision to Diff 311190.Dec 11 2020, 5:45 AM

Rebase

jansvoboda11 retitled this revision from [clang][cli] Don't always emit -f[no-]experimental-new-pass-manager to [clang][cli] [clang][cli] Don't always emit -f[no-]legacy-pass-manager.Dec 11 2020, 5:45 AM
Harbormaster completed remote builds in B82019: Diff 311190.Dec 11 2020, 5:46 AM
jansvoboda11 updated this revision to Diff 311193.Dec 11 2020, 5:49 AM

Extract default value

Harbormaster completed remote builds in B82022: Diff 311193.Dec 11 2020, 5:50 AM
jansvoboda11 updated this revision to Diff 311201.Dec 11 2020, 5:57 AM

Rebase

Harbormaster completed remote builds in B82027: Diff 311201.Dec 11 2020, 5:59 AM
jansvoboda11 retitled this revision from [clang][cli] [clang][cli] Don't always emit -f[no-]legacy-pass-manager to [clang][cli] Don't always emit -f[no-]legacy-pass-manager.Dec 11 2020, 6:26 AM
dexonsmith accepted this revision.Dec 11 2020, 2:13 PM

LGTM too.

This revision was landed with ongoing or failed builds.Dec 12 2020, 1:12 AM
Closed by commit rG10f40576f7b4: [clang][cli] Don't always emit -f[no-]legacy-pass-manager (authored by jansvoboda11). · Explain Why
This revision was automatically updated to reflect the committed changes.
jansvoboda11 added a commit: rG10f40576f7b4: [clang][cli] Don't always emit -f[no-]legacy-pass-manager.

Revision Contents

PathSize
clang/
unittests/
Frontend/
4 lines
llvm/
include/
llvm/
Option/
1 line

Diff 311370

clang/unittests/Frontend/CompilerInvocationTest.cpp

Show First 20 Lines • Show All 192 Lines • ▼ Show 20 LinesTEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentNone) {
CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags); CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
ASSERT_FALSE(Diags->hasErrorOccurred()); ASSERT_FALSE(Diags->hasErrorOccurred());
ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, PassManagerDefault); ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, PassManagerDefault);
Invocation.generateCC1CommandLine(GeneratedArgs, *this); Invocation.generateCC1CommandLine(GeneratedArgs, *this);
ASSERT_THAT(GeneratedArgs, Contains(StrEq(PassManagerResetByFlag))); ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag))));
ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag)))); ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag))));
} }
TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentChange) { TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentChange) {
const char *Args[] = {PassManagerChangedByFlag}; const char *Args[] = {PassManagerChangedByFlag};
CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags); CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
ASSERT_FALSE(Diags->hasErrorOccurred()); ASSERT_FALSE(Diags->hasErrorOccurred());
ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, !PassManagerDefault); ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, !PassManagerDefault);
Invocation.generateCC1CommandLine(GeneratedArgs, *this); Invocation.generateCC1CommandLine(GeneratedArgs, *this);
ASSERT_THAT(GeneratedArgs, Contains(StrEq(PassManagerChangedByFlag))); ASSERT_THAT(GeneratedArgs, Contains(StrEq(PassManagerChangedByFlag)));
ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag)))); ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag))));
} }
TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentReset) { TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentReset) {
const char *Args[] = {PassManagerResetByFlag}; const char *Args[] = {PassManagerResetByFlag};
CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags); CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
ASSERT_FALSE(Diags->hasErrorOccurred()); ASSERT_FALSE(Diags->hasErrorOccurred());
ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, PassManagerDefault); ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, PassManagerDefault);
Invocation.generateCC1CommandLine(GeneratedArgs, *this); Invocation.generateCC1CommandLine(GeneratedArgs, *this);
ASSERT_THAT(GeneratedArgs, Contains(StrEq(PassManagerResetByFlag))); ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag))));
ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag)))); ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag))));
} }
TEST_F(CommandLineTest, CanGenerateCC1CommandLineFlag) { TEST_F(CommandLineTest, CanGenerateCC1CommandLineFlag) {
const char *Args[] = {"-fmodules-strict-context-hash"}; const char *Args[] = {"-fmodules-strict-context-hash"};
CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags); CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
▲ Show 20 Lines • Show All 187 Lines • Show Last 20 Lines

llvm/include/llvm/Option/OptParser.td

Show First 20 Lines • Show All 167 Lines • ▼ Show 20 Linesclass MarshallingInfoBitfieldFlag<code keypath, code value>
: MarshallingInfoFlag<keypath, "0u"> { : MarshallingInfoFlag<keypath, "0u"> {
code Normalizer = "makeFlagToValueNormalizer("#value#")"; code Normalizer = "makeFlagToValueNormalizer("#value#")";
code ValueMerger = "mergeMaskValue"; code ValueMerger = "mergeMaskValue";
code ValueExtractor = "(extractMaskValue<unsigned, decltype("#value#"), "#value#">)"; code ValueExtractor = "(extractMaskValue<unsigned, decltype("#value#"), "#value#">)";
} }
class MarshallingInfoBooleanFlag<code keypath, code defaultvalue, code neg_name, string neg_spelling> class MarshallingInfoBooleanFlag<code keypath, code defaultvalue, code neg_name, string neg_spelling>
: MarshallingInfoFlag<keypath, defaultvalue> { : MarshallingInfoFlag<keypath, defaultvalue> {
bit ShouldAlwaysEmit = 1;
code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")"; code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
code Denormalizer = "makeBooleanFlagDenormalizer(\""#neg_spelling#"\")"; code Denormalizer = "makeBooleanFlagDenormalizer(\""#neg_spelling#"\")";
} }
// Mixins for additional marshalling attributes. // Mixins for additional marshalling attributes.
class IsNegative { class IsNegative {
code Normalizer = "normalizeSimpleNegativeFlag"; code Normalizer = "normalizeSimpleNegativeFlag";
Show All 23 Lines