Skip to content

[flang][driver] Introduce FCC_OVERRIDE_OPTIONS. #140556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
abidh merged 9 commits into from
Jun 2, 2025
Merged

[flang][driver] Introduce FCC_OVERRIDE_OPTIONS. #140556

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5d20af4
[flang][driver] Introduce FCC_OVERRIDE_OPTIONS.
abidh May 19, 2025
d1f2c9b
Add documentation for FCC_OVERRIDE_OPTIONS.
abidh May 29, 2025
d093a6a
Mention that effect on options added by the config files.
abidh May 29, 2025
9faf4d3
Add a test for option from config file.
abidh May 29, 2025
db45474
Handle review comments.
abidh May 30, 2025
2efd653
Handle review comments.
abidh May 30, 2025
f41f8ff
Replace -target with --target= to address review comments.
abidh May 30, 2025
de566e3
Make it clear that ^ adds argument adter compiler name.
abidh Jun 1, 2025
3345334
Empty commit to trigger build
abidh Jun 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/include/clang/Driver/Driver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,7 @@ llvm::Error expandResponseFiles(SmallVectorImpl<const char *> &Args,
/// See applyOneOverrideOption.
void applyOverrideOptions(SmallVectorImpl<const char *> &Args,
const char *OverrideOpts,
llvm::StringSet<> &SavedStrings,
llvm::StringSet<> &SavedStrings, StringRef EnvVar,
raw_ostream *OS = nullptr);

} // end namespace driver
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Driver/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7289,7 +7289,7 @@ static void applyOneOverrideOption(raw_ostream &OS,
void driver::applyOverrideOptions(SmallVectorImpl<const char *> &Args,
const char *OverrideStr,
llvm::StringSet<> &SavedStrings,
raw_ostream *OS) {
StringRef EnvVar, raw_ostream *OS) {
if (!OS)
OS = &llvm::nulls();

Expand All @@ -7298,7 +7298,7 @@ void driver::applyOverrideOptions(SmallVectorImpl<const char *> &Args,
OS = &llvm::nulls();
}

*OS << "### CCC_OVERRIDE_OPTIONS: " << OverrideStr << "\n";
*OS << "### " << EnvVar << ": " << OverrideStr << "\n";

// This does not need to be efficient.

Expand Down
2 changes: 1 addition & 1 deletion clang/tools/driver/driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) {
if (const char *OverrideStr = ::getenv("CCC_OVERRIDE_OPTIONS")) {
// FIXME: Driver shouldn't take extra initial argument.
driver::applyOverrideOptions(Args, OverrideStr, SavedStrings,
&llvm::errs());
"CCC_OVERRIDE_OPTIONS", &llvm::errs());
}

std::string Path = GetExecutablePath(ToolContext.Path, CanonicalPrefixes);
Expand Down
12 changes: 12 additions & 0 deletions flang/test/Driver/fcc_override.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
! RUN: env FCC_OVERRIDE_OPTIONS="#+-Os +-Oz +-O +-O3 +-Oignore +a +b +c xb Xa Omagic ^-### " %flang -target x86_64-unknown-linux-gnu %s -O2 b -O3 2>&1 | FileCheck %s
Copy link
Contributor

@kiranchandramohan kiranchandramohan May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does +, x,X have special meanings? Is that documented anywhere?

I think we should document FCC_OVERRIDE_OPTIONS. One possible location is https://github.com/llvm/llvm-project/blob/main/flang/docs/FlangDriver.md

Copy link
Contributor Author

@abidh abidh May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, those have special meaning and they were only documented in the comments. I have now added documentation for it in the suggested file.

Copy link
Member

@MaskRay MaskRay May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--target=x86_64-unknown-linux-gnu instead of the long deprecated -target

Copy link
Contributor

@tarunprabhu tarunprabhu May 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also be --target?

Copy link
Contributor Author

@abidh abidh May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Thanks for catching this. Fixed now.

! RUN: env FCC_OVERRIDE_OPTIONS="x-Werror +-g" %flang -target x86_64-unknown-linux-gnu -Werror %s -c -### 2>&1 | FileCheck %s -check-prefix=RM-WERROR

! CHECK: "-fc1"
! CHECK-NOT: "-Oignore"
! CHECK: "-Omagic"
! CHECK-NOT: "-Oignore"

! RM-WERROR: ### FCC_OVERRIDE_OPTIONS: x-Werror +-g
! RM-WERROR-NEXT: ### Deleting argument -Werror
! RM-WERROR-NEXT: ### Adding argument -g at end
! RM-WERROR-NOT: "-Werror"
7 changes: 7 additions & 0 deletions flang/tools/flang-driver/driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ int main(int argc, const char **argv) {
}
}

llvm::StringSet<> SavedStrings;
Copy link
Contributor

@kiranchandramohan kiranchandramohan May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
llvm::StringSet<> SavedStrings;
llvm::StringSet<> savedStrings;

// Handle FCC_OVERRIDE_OPTIONS, used for editing a command line behind the
// scenes.
if (const char *OverrideStr = ::getenv("FCC_OVERRIDE_OPTIONS"))
Copy link
Contributor

@kiranchandramohan kiranchandramohan May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (const char *OverrideStr = ::getenv("FCC_OVERRIDE_OPTIONS"))
if (const char *overrideStr = ::getenv("FCC_OVERRIDE_OPTIONS"))

clang::driver::applyOverrideOptions(args, OverrideStr, SavedStrings,
"FCC_OVERRIDE_OPTIONS", &llvm::errs());

// Not in the frontend mode - continue in the compiler driver mode.

// Create DiagnosticsEngine for the compiler driver
Expand Down