[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
banach-space wrote:
> I don't feel strongly about it
ACK.
I mostly care about consistency in the interface exposed to the end-user. TBH,
I've never really investigated the relationship of `-O{0|1|2|3|4}` with various
feature flags. What I described definitely holds for `-f{no-}some_feature`
flags. But not 100% sure about `-O{0|1|2|3|4}`.
TBH, this feels like just too much control in users' hands. So I would keep
this option as hidden and use strictly for compiler development.
You could also just update the [relevant help
text](https://github.com/banach-space/llvm-project/blob/fecd909bdd66ab1942af9cda6b67d82b5ef016ae/clang/include/clang/Driver/Options.td#L6337-L6338):
```
PosFlag,
NegFlag>;
```
Btw, thanks for working on this - really great to see progress on this front!
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
https://github.com/tblah closed https://github.com/llvm/llvm-project/pull/73111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
kiranchandramohan wrote:
> That's fine, but then one has to decide whether -f{no}-alias-analysis
> overrides -O{n} or not? I think that "explicit" request from a user should
> always take precedence. This leads to (pseudo code):
>
> opts.AliasAnalysis = 0;
> if (opt level requiring alias analysis)
> opts.AliasAnalysis = 1;
>
> / / User request takes precedence when it comes to alias analysis.
> if (-falias-analysis or -fno-alias-analysis) then
> "do whatever the user requested"
>Separately, could you check what Clang does and make sure that that would be
>consistent?
@banach-space This is exactly the handling in the front-end driver as given
below (and in `lib/Frontend/CompilerInvocation`). The flang driver is only
deciding whether to forward or not.
```
opts.AliasAnalysis = opts.OptimizationLevel > 0;
if (auto *arg =
args.getLastArg(clang::driver::options::OPT_falias_analysis,
clang::driver::options::OPT_fno_alias_analysis))
opts.AliasAnalysis =
arg->getOption().matches(clang::driver::options::OPT_falias_analysis);
```
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
https://github.com/kiranchandramohan approved this pull request. LGTM. https://github.com/llvm/llvm-project/pull/73111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -2,12 +2,25 @@ ! See flang/test/Fir/tbaa-codegen.fir for a test that the output is correct ! RUN: %flang -c -emit-llvm -falias-analysis %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL -! RUN: %flang -c -emit-llvm -falias-analysis -fno-alias-analysis %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL +! RUN: %flang -c -emit-llvm -Ofast %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL +! RUN: %flang -c -emit-llvm -O3 %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL +! RUN: %flang -c -emit-llvm -O2 %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL +! RUN: %flang -c -emit-llvm -O1 %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL + +! RUN: %flang -c -emit-llvm -O0 %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL +! RUN: %flang -c -emit-llvm -Ofast -fno-alias-analysis %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL tblah wrote: I have added a test in this case. `-fno-alias-analysis` takes precedence over `-Ofast` no matter their order. I don't think there is anything in clang that is equivalent to `-f[no-]alias-analysis`. These flags were added to flang so we could upstream alias analysis without enabling it by default. I would like to keep them after enabling by default in case alias analysis leads to any regressions. https://github.com/llvm/llvm-project/pull/73111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -142,6 +142,24 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
+ if (aliasAnalysis) {
+bool faliasAnalysis =
+aliasAnalysis->getOption().matches(options::OPT_falias_analysis);
+// only pass on the argument if it does not match that implied by the
+// optimization level
+if (optLevel && !faliasAnalysis) {
+ CmdArgs.push_back("-fno-alias-analysis");
+} else {
+ if (faliasAnalysis)
+// requested alias analysis but no optimization enabled
+CmdArgs.push_back("-falias-analysis");
+}
+ }
banach-space wrote:
[nit] It would be worth adding a comment here explaining the intent (e.g. "Only
forward `-f{no}-alias-analysis if that makes sense considering other
optimisation flags"). Alternatively, you could rename `optLevel` as
`optLevelForSpeed` to communicate that. Or both.
Just to make things clear for our future selves ;-)
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -2,12 +2,25 @@ ! See flang/test/Fir/tbaa-codegen.fir for a test that the output is correct ! RUN: %flang -c -emit-llvm -falias-analysis %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL -! RUN: %flang -c -emit-llvm -falias-analysis -fno-alias-analysis %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL +! RUN: %flang -c -emit-llvm -Ofast %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL +! RUN: %flang -c -emit-llvm -O3 %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL +! RUN: %flang -c -emit-llvm -O2 %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL +! RUN: %flang -c -emit-llvm -O1 %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL + +! RUN: %flang -c -emit-llvm -O0 %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL +! RUN: %flang -c -emit-llvm -Ofast -fno-alias-analysis %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL banach-space wrote: What's going to happen in this case: ``` ! RUN: %flang -c -emit-llvm -fno-alias-analysis -Ofast %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL ``` ? And will that behavior be consistent with Clang? https://github.com/llvm/llvm-project/pull/73111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -242,10 +242,24 @@ static void
parseCodeGenArgs(Fortran::frontend::CodeGenOptions ,
clang::driver::options::OPT_fno_loop_versioning, false))
opts.LoopVersioning = 1;
- opts.AliasAnalysis =
- args.hasFlag(clang::driver::options::OPT_falias_analysis,
- clang::driver::options::OPT_fno_alias_analysis,
- /*default=*/false);
+ bool aliasAnalysis = false;
+ bool noAliasAnalysis = false;
+ if (auto *arg =
+ args.getLastArg(clang::driver::options::OPT_falias_analysis,
+ clang::driver::options::OPT_fno_alias_analysis)) {
+if (arg->getOption().matches(clang::driver::options::OPT_falias_analysis))
+ aliasAnalysis = true;
+else
+ noAliasAnalysis = true;
+ }
+ opts.AliasAnalysis = 0;
+ if (opts.OptimizationLevel > 0) {
+if (!noAliasAnalysis)
+ opts.AliasAnalysis = 1;
+ } else {
+if (aliasAnalysis)
+ opts.AliasAnalysis = 1;
+ }
banach-space wrote:
> I don't really understand how that differs from what I already have
1. It removes the need for `aliasAnalysis` and `noAliasAnalysis`.
1.1 I also hinted elsewhere
(https://github.com/llvm/llvm-project/pull/73111#discussion_r1402712333) that
we should only need 1 bool to represent the same logic - an option is either
enabled or disabled and one bool should be sufficient to represent that.
1.2 Fewer variables leads to lower cognitive load, which is preferred.
2. No nested `if`s are required (i.e. `if (opts.OptimizationLevel > 0) { if
(!noAliasAnalysis)) {}}`).
3. The logic for processing optimization options (e.g. `-O4`) and alias
analysis options (e.g. `-falias-analysis`) is clearly separated.
4. From what I can tell, the number of source lines drops (though hard to tell
for sure with GitHub's formatting).
I might be missing something though, so please correct me if I'm wrong and what
I'm proposing wouldn't work.
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
https://github.com/tblah edited https://github.com/llvm/llvm-project/pull/73111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/73111
>From 617d6d23b2f861cd6dceb82f54a2685059b6 Mon Sep 17 00:00:00 2001
From: Tom Eccles
Date: Thu, 14 Sep 2023 09:09:29 +
Subject: [PATCH 1/6] [flang] Enable alias tags pass by default
Enable by default when optimizing for speed.
For simplicity, only forward the flag to the frontend driver when it
contradicts what is implied by the optimization level.
---
clang/lib/Driver/ToolChains/Flang.cpp | 20
flang/include/flang/Tools/CLOptions.inc | 8
flang/lib/Frontend/CompilerInvocation.cpp | 22 ++
flang/test/Driver/falias-analysis.f90 | 4
flang/test/Driver/mlir-pass-pipeline.f90 | 2 ++
flang/test/Driver/optimization-remark.f90 | 22 +-
flang/test/Fir/basic-program.fir | 4
flang/tools/tco/tco.cpp | 1 +
8 files changed, 62 insertions(+), 21 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp
b/clang/lib/Driver/ToolChains/Flang.cpp
index 8bdd920c3dcbb79..9382433b94dadfd 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
+ if (aliasAnalysis) {
+bool falias_analysis =
+aliasAnalysis->getOption().matches(options::OPT_falias_analysis);
+// only pass on the argument if it does not match that implied by the
+// optimization level
+if (optLevel) {
+ if (!falias_analysis) {
+CmdArgs.push_back("-fno-alias-analysis");
+ }
+} else {
+ if (falias_analysis)
+// requested alias analysis but no optimization enabled
+CmdArgs.push_back("-falias-analysis");
+}
+ }
+
Args.addAllArgs(CmdArgs, {options::OPT_flang_experimental_hlfir,
options::OPT_flang_deprecated_no_hlfir,
options::OPT_flang_experimental_polymorphism,
diff --git a/flang/include/flang/Tools/CLOptions.inc
b/flang/include/flang/Tools/CLOptions.inc
index c452c023b4a80ce..5a17385fb3dae87 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -157,11 +157,11 @@ inline void addDebugFoundationPass(mlir::PassManager )
{
[&]() { return fir::createAddDebugFoundationPass(); });
}
-inline void addFIRToLLVMPass(
-mlir::PassManager , llvm::OptimizationLevel optLevel = defaultOptLevel)
{
+inline void addFIRToLLVMPass(mlir::PassManager ,
+llvm::OptimizationLevel optLevel = defaultOptLevel, bool applyTbaa = true)
{
fir::FIRToLLVMPassOptions options;
options.ignoreMissingTypeDescriptors = ignoreMissingTypeDescriptors;
- options.applyTBAA = optLevel.isOptimizingForSpeed();
+ options.applyTBAA = applyTbaa;
options.forceUnifiedTBAATree = useOldAliasTags;
addPassConditionally(pm, disableFirToLlvmIr,
[&]() { return fir::createFIRToLLVMPass(options); });
@@ -311,7 +311,7 @@ inline void createDefaultFIRCodeGenPassPipeline(
if (config.VScaleMin != 0)
pm.addPass(fir::createVScaleAttrPass({config.VScaleMin,
config.VScaleMax}));
- fir::addFIRToLLVMPass(pm, config.OptLevel);
+ fir::addFIRToLLVMPass(pm, config.OptLevel, config.AliasAnalysis);
}
/// Create a pass pipeline for lowering from MLIR to LLVM IR
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp
b/flang/lib/Frontend/CompilerInvocation.cpp
index cb4f2d6a6225205..cfb1dd91ead3056 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -242,10 +242,24 @@ static void
parseCodeGenArgs(Fortran::frontend::CodeGenOptions ,
clang::driver::options::OPT_fno_loop_versioning, false))
opts.LoopVersioning = 1;
- opts.AliasAnalysis =
- args.hasFlag(clang::driver::options::OPT_falias_analysis,
- clang::driver::options::OPT_fno_alias_analysis,
- /*default=*/false);
+ bool aliasAnalysis = false;
+ bool noAliasAnalysis = false;
+ if (auto *arg =
+ args.getLastArg(clang::driver::options::OPT_falias_analysis,
+ clang::driver::options::OPT_fno_alias_analysis)) {
+if (arg->getOption().matches(clang::driver::options::OPT_falias_analysis))
+ aliasAnalysis = true;
+else
+ noAliasAnalysis = true;
+ }
+ opts.AliasAnalysis = 0;
+ if (opts.OptimizationLevel > 0) {
+if (!noAliasAnalysis)
+ opts.AliasAnalysis = 1;
+ } else {
+if (aliasAnalysis)
+ opts.AliasAnalysis = 1;
+ }
for (auto *a : args.filtered(clang::driver::options::OPT_fpass_plugin_EQ))
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
tblah wrote:
It turns out flang doesn't support `-Os`. Probably a bug here
https://github.com/llvm/llvm-project/blob/main/flang/lib/Frontend/CompilerInvocation.cpp#L110
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -242,10 +242,24 @@ static void
parseCodeGenArgs(Fortran::frontend::CodeGenOptions ,
clang::driver::options::OPT_fno_loop_versioning, false))
opts.LoopVersioning = 1;
- opts.AliasAnalysis =
- args.hasFlag(clang::driver::options::OPT_falias_analysis,
- clang::driver::options::OPT_fno_alias_analysis,
- /*default=*/false);
+ bool aliasAnalysis = false;
+ bool noAliasAnalysis = false;
+ if (auto *arg =
+ args.getLastArg(clang::driver::options::OPT_falias_analysis,
+ clang::driver::options::OPT_fno_alias_analysis)) {
+if (arg->getOption().matches(clang::driver::options::OPT_falias_analysis))
+ aliasAnalysis = true;
+else
+ noAliasAnalysis = true;
+ }
+ opts.AliasAnalysis = 0;
+ if (opts.OptimizationLevel > 0) {
+if (!noAliasAnalysis)
+ opts.AliasAnalysis = 1;
+ } else {
+if (aliasAnalysis)
+ opts.AliasAnalysis = 1;
+ }
tblah wrote:
I don't really understand how that differs from what I already have. But sure
I'll do it if you think it is clearer.
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -242,10 +242,24 @@ static void
parseCodeGenArgs(Fortran::frontend::CodeGenOptions ,
clang::driver::options::OPT_fno_loop_versioning, false))
opts.LoopVersioning = 1;
- opts.AliasAnalysis =
- args.hasFlag(clang::driver::options::OPT_falias_analysis,
- clang::driver::options::OPT_fno_alias_analysis,
- /*default=*/false);
+ bool aliasAnalysis = false;
+ bool noAliasAnalysis = false;
+ if (auto *arg =
+ args.getLastArg(clang::driver::options::OPT_falias_analysis,
+ clang::driver::options::OPT_fno_alias_analysis)) {
+if (arg->getOption().matches(clang::driver::options::OPT_falias_analysis))
+ aliasAnalysis = true;
+else
+ noAliasAnalysis = true;
+ }
+ opts.AliasAnalysis = 0;
+ if (opts.OptimizationLevel > 0) {
+if (!noAliasAnalysis)
+ opts.AliasAnalysis = 1;
+ } else {
+if (aliasAnalysis)
+ opts.AliasAnalysis = 1;
+ }
banach-space wrote:
It won't if you implement this logic:
https://github.com/llvm/llvm-project/pull/73111#discussion_r1403215158
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -242,10 +242,24 @@ static void
parseCodeGenArgs(Fortran::frontend::CodeGenOptions ,
clang::driver::options::OPT_fno_loop_versioning, false))
opts.LoopVersioning = 1;
- opts.AliasAnalysis =
- args.hasFlag(clang::driver::options::OPT_falias_analysis,
- clang::driver::options::OPT_fno_alias_analysis,
- /*default=*/false);
+ bool aliasAnalysis = false;
+ bool noAliasAnalysis = false;
+ if (auto *arg =
+ args.getLastArg(clang::driver::options::OPT_falias_analysis,
+ clang::driver::options::OPT_fno_alias_analysis)) {
+if (arg->getOption().matches(clang::driver::options::OPT_falias_analysis))
+ aliasAnalysis = true;
+else
+ noAliasAnalysis = true;
+ }
+ opts.AliasAnalysis = 0;
+ if (opts.OptimizationLevel > 0) {
+if (!noAliasAnalysis)
+ opts.AliasAnalysis = 1;
+ } else {
+if (aliasAnalysis)
+ opts.AliasAnalysis = 1;
+ }
tblah wrote:
Then opts.AliasAnalysis would be false whenever -falias-analysis is not
present. We want to enable it by default when optimizing.
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
banach-space wrote:
> But I want there to still be a separate flag available to override this
> default behavior.
That's fine, but then one has to decide whether `-f{no}-alias-analysis`
overrides `-O{n}` or not? I think that "explicit" request from a user should
always take precedence. This leads to (pseudo code):
```
opts.AliasAnalysis = 0;
if (opt level requiring alias analysis)
opts.AliasAnalysis = 1;
// User request takes precedence when it comes to alias analysis.
if (-falias-analysis or -fno-alias-analysis) then
"do whatever the user requested"
```
Separately, could you check what Clang does and make sure that that would be
consistent?
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/73111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -242,10 +242,24 @@ static void
parseCodeGenArgs(Fortran::frontend::CodeGenOptions ,
clang::driver::options::OPT_fno_loop_versioning, false))
opts.LoopVersioning = 1;
- opts.AliasAnalysis =
- args.hasFlag(clang::driver::options::OPT_falias_analysis,
- clang::driver::options::OPT_fno_alias_analysis,
- /*default=*/false);
+ bool aliasAnalysis = false;
+ bool noAliasAnalysis = false;
+ if (auto *arg =
+ args.getLastArg(clang::driver::options::OPT_falias_analysis,
+ clang::driver::options::OPT_fno_alias_analysis)) {
+if (arg->getOption().matches(clang::driver::options::OPT_falias_analysis))
+ aliasAnalysis = true;
+else
+ noAliasAnalysis = true;
+ }
+ opts.AliasAnalysis = 0;
+ if (opts.OptimizationLevel > 0) {
+if (!noAliasAnalysis)
+ opts.AliasAnalysis = 1;
+ } else {
+if (aliasAnalysis)
+ opts.AliasAnalysis = 1;
+ }
banach-space wrote:
```suggestion
if (auto *arg =
args.getLastArg(clang::driver::options::OPT_falias_analysis,
clang::driver::options::OPT_fno_alias_analysis)) {
opts.AliasAnalysis =
(arg->getOption().matches(clang::driver::options::OPT_falias_analysis)) ? : 0;
}
```
Why wouldn't this work?
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
tblah wrote: > Thank you for the changes, Tom! > > I have one minor comment, but I would like to ask to merge this after US > holidays, if possible. Could you please postpone the merging until Monday GMT? Sure. I'll wait until Monday. https://github.com/llvm/llvm-project/pull/73111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
vzakhari wrote:
It looks like `clang` generates `tbaa` metadata at all opt levels, except
`-O0`. I think this makes sense: the optimization themselves need to decide
how to use it, e.g. for improving performance/code-size/etc.
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
https://github.com/vzakhari commented: Thank you for the changes, Tom! I have one minor comment, but I would like to ask to merge this after US holidays, if possible. Could you please postpone the merging until Monday GMT? https://github.com/llvm/llvm-project/pull/73111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/73111
>From 617d6d23b2f861cd6dceb82f54a2685059b6 Mon Sep 17 00:00:00 2001
From: Tom Eccles
Date: Thu, 14 Sep 2023 09:09:29 +
Subject: [PATCH 1/3] [flang] Enable alias tags pass by default
Enable by default when optimizing for speed.
For simplicity, only forward the flag to the frontend driver when it
contradicts what is implied by the optimization level.
---
clang/lib/Driver/ToolChains/Flang.cpp | 20
flang/include/flang/Tools/CLOptions.inc | 8
flang/lib/Frontend/CompilerInvocation.cpp | 22 ++
flang/test/Driver/falias-analysis.f90 | 4
flang/test/Driver/mlir-pass-pipeline.f90 | 2 ++
flang/test/Driver/optimization-remark.f90 | 22 +-
flang/test/Fir/basic-program.fir | 4
flang/tools/tco/tco.cpp | 1 +
8 files changed, 62 insertions(+), 21 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp
b/clang/lib/Driver/ToolChains/Flang.cpp
index 8bdd920c3dcbb79..9382433b94dadfd 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
+ if (aliasAnalysis) {
+bool falias_analysis =
+aliasAnalysis->getOption().matches(options::OPT_falias_analysis);
+// only pass on the argument if it does not match that implied by the
+// optimization level
+if (optLevel) {
+ if (!falias_analysis) {
+CmdArgs.push_back("-fno-alias-analysis");
+ }
+} else {
+ if (falias_analysis)
+// requested alias analysis but no optimization enabled
+CmdArgs.push_back("-falias-analysis");
+}
+ }
+
Args.addAllArgs(CmdArgs, {options::OPT_flang_experimental_hlfir,
options::OPT_flang_deprecated_no_hlfir,
options::OPT_flang_experimental_polymorphism,
diff --git a/flang/include/flang/Tools/CLOptions.inc
b/flang/include/flang/Tools/CLOptions.inc
index c452c023b4a80ce..5a17385fb3dae87 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -157,11 +157,11 @@ inline void addDebugFoundationPass(mlir::PassManager )
{
[&]() { return fir::createAddDebugFoundationPass(); });
}
-inline void addFIRToLLVMPass(
-mlir::PassManager , llvm::OptimizationLevel optLevel = defaultOptLevel)
{
+inline void addFIRToLLVMPass(mlir::PassManager ,
+llvm::OptimizationLevel optLevel = defaultOptLevel, bool applyTbaa = true)
{
fir::FIRToLLVMPassOptions options;
options.ignoreMissingTypeDescriptors = ignoreMissingTypeDescriptors;
- options.applyTBAA = optLevel.isOptimizingForSpeed();
+ options.applyTBAA = applyTbaa;
options.forceUnifiedTBAATree = useOldAliasTags;
addPassConditionally(pm, disableFirToLlvmIr,
[&]() { return fir::createFIRToLLVMPass(options); });
@@ -311,7 +311,7 @@ inline void createDefaultFIRCodeGenPassPipeline(
if (config.VScaleMin != 0)
pm.addPass(fir::createVScaleAttrPass({config.VScaleMin,
config.VScaleMax}));
- fir::addFIRToLLVMPass(pm, config.OptLevel);
+ fir::addFIRToLLVMPass(pm, config.OptLevel, config.AliasAnalysis);
}
/// Create a pass pipeline for lowering from MLIR to LLVM IR
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp
b/flang/lib/Frontend/CompilerInvocation.cpp
index cb4f2d6a6225205..cfb1dd91ead3056 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -242,10 +242,24 @@ static void
parseCodeGenArgs(Fortran::frontend::CodeGenOptions ,
clang::driver::options::OPT_fno_loop_versioning, false))
opts.LoopVersioning = 1;
- opts.AliasAnalysis =
- args.hasFlag(clang::driver::options::OPT_falias_analysis,
- clang::driver::options::OPT_fno_alias_analysis,
- /*default=*/false);
+ bool aliasAnalysis = false;
+ bool noAliasAnalysis = false;
+ if (auto *arg =
+ args.getLastArg(clang::driver::options::OPT_falias_analysis,
+ clang::driver::options::OPT_fno_alias_analysis)) {
+if (arg->getOption().matches(clang::driver::options::OPT_falias_analysis))
+ aliasAnalysis = true;
+else
+ noAliasAnalysis = true;
+ }
+ opts.AliasAnalysis = 0;
+ if (opts.OptimizationLevel > 0) {
+if (!noAliasAnalysis)
+ opts.AliasAnalysis = 1;
+ } else {
+if (aliasAnalysis)
+ opts.AliasAnalysis = 1;
+ }
for (auto *a : args.filtered(clang::driver::options::OPT_fpass_plugin_EQ))
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
tblah wrote:
I suppose alias analysis could make sense at `-Os` too, because it could enable
better common sub-expression elimination and hoisting. What do you think?
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -242,10 +242,24 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions , clang::driver::options::OPT_fno_loop_versioning, false)) opts.LoopVersioning = 1; - opts.AliasAnalysis = - args.hasFlag(clang::driver::options::OPT_falias_analysis, - clang::driver::options::OPT_fno_alias_analysis, - /*default=*/false); + bool aliasAnalysis = false; + bool noAliasAnalysis = false; banach-space wrote: Why do we need two bools to model one thing? What's the logic that we trying to model here? https://github.com/llvm/llvm-project/pull/73111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
+ if (aliasAnalysis) {
+bool falias_analysis =
banach-space wrote:
```suggestion
bool faliasAnalysis =
```
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
+ if (aliasAnalysis) {
+bool falias_analysis =
+aliasAnalysis->getOption().matches(options::OPT_falias_analysis);
+// only pass on the argument if it does not match that implied by the
+// optimization level
+if (optLevel) {
+ if (!falias_analysis) {
+CmdArgs.push_back("-fno-alias-analysis");
+ }
+} else {
banach-space wrote:
```suggestion
if (optLevel && !falias_analysis) {
CmdArgs.push_back("-fno-alias-analysis");
}
```
?
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
banach-space wrote:
What about other opt levels? Do we enable or disable alias analysis for these
opt levels?
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
https://github.com/banach-space edited https://github.com/llvm/llvm-project/pull/73111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
https://github.com/martinboehme updated
https://github.com/llvm/llvm-project/pull/73111
>From 617d6d23b2f861cd6dceb82f54a2685059b6 Mon Sep 17 00:00:00 2001
From: Tom Eccles
Date: Thu, 14 Sep 2023 09:09:29 +
Subject: [PATCH 1/2] [flang] Enable alias tags pass by default
Enable by default when optimizing for speed.
For simplicity, only forward the flag to the frontend driver when it
contradicts what is implied by the optimization level.
---
clang/lib/Driver/ToolChains/Flang.cpp | 20
flang/include/flang/Tools/CLOptions.inc | 8
flang/lib/Frontend/CompilerInvocation.cpp | 22 ++
flang/test/Driver/falias-analysis.f90 | 4
flang/test/Driver/mlir-pass-pipeline.f90 | 2 ++
flang/test/Driver/optimization-remark.f90 | 22 +-
flang/test/Fir/basic-program.fir | 4
flang/tools/tco/tco.cpp | 1 +
8 files changed, 62 insertions(+), 21 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp
b/clang/lib/Driver/ToolChains/Flang.cpp
index 8bdd920c3dcbb79..9382433b94dadfd 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList ,
if (shouldLoopVersion(Args))
CmdArgs.push_back("-fversion-loops-for-stride");
+ Arg *aliasAnalysis = Args.getLastArg(options::OPT_falias_analysis,
+ options::OPT_fno_alias_analysis);
+ Arg *optLevel =
+ Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
+ if (aliasAnalysis) {
+bool falias_analysis =
+aliasAnalysis->getOption().matches(options::OPT_falias_analysis);
+// only pass on the argument if it does not match that implied by the
+// optimization level
+if (optLevel) {
+ if (!falias_analysis) {
+CmdArgs.push_back("-fno-alias-analysis");
+ }
+} else {
+ if (falias_analysis)
+// requested alias analysis but no optimization enabled
+CmdArgs.push_back("-falias-analysis");
+}
+ }
+
Args.addAllArgs(CmdArgs, {options::OPT_flang_experimental_hlfir,
options::OPT_flang_deprecated_no_hlfir,
options::OPT_flang_experimental_polymorphism,
diff --git a/flang/include/flang/Tools/CLOptions.inc
b/flang/include/flang/Tools/CLOptions.inc
index c452c023b4a80ce..5a17385fb3dae87 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -157,11 +157,11 @@ inline void addDebugFoundationPass(mlir::PassManager )
{
[&]() { return fir::createAddDebugFoundationPass(); });
}
-inline void addFIRToLLVMPass(
-mlir::PassManager , llvm::OptimizationLevel optLevel = defaultOptLevel)
{
+inline void addFIRToLLVMPass(mlir::PassManager ,
+llvm::OptimizationLevel optLevel = defaultOptLevel, bool applyTbaa = true)
{
fir::FIRToLLVMPassOptions options;
options.ignoreMissingTypeDescriptors = ignoreMissingTypeDescriptors;
- options.applyTBAA = optLevel.isOptimizingForSpeed();
+ options.applyTBAA = applyTbaa;
options.forceUnifiedTBAATree = useOldAliasTags;
addPassConditionally(pm, disableFirToLlvmIr,
[&]() { return fir::createFIRToLLVMPass(options); });
@@ -311,7 +311,7 @@ inline void createDefaultFIRCodeGenPassPipeline(
if (config.VScaleMin != 0)
pm.addPass(fir::createVScaleAttrPass({config.VScaleMin,
config.VScaleMax}));
- fir::addFIRToLLVMPass(pm, config.OptLevel);
+ fir::addFIRToLLVMPass(pm, config.OptLevel, config.AliasAnalysis);
}
/// Create a pass pipeline for lowering from MLIR to LLVM IR
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp
b/flang/lib/Frontend/CompilerInvocation.cpp
index cb4f2d6a6225205..cfb1dd91ead3056 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -242,10 +242,24 @@ static void
parseCodeGenArgs(Fortran::frontend::CodeGenOptions ,
clang::driver::options::OPT_fno_loop_versioning, false))
opts.LoopVersioning = 1;
- opts.AliasAnalysis =
- args.hasFlag(clang::driver::options::OPT_falias_analysis,
- clang::driver::options::OPT_fno_alias_analysis,
- /*default=*/false);
+ bool aliasAnalysis = false;
+ bool noAliasAnalysis = false;
+ if (auto *arg =
+ args.getLastArg(clang::driver::options::OPT_falias_analysis,
+ clang::driver::options::OPT_fno_alias_analysis)) {
+if (arg->getOption().matches(clang::driver::options::OPT_falias_analysis))
+ aliasAnalysis = true;
+else
+ noAliasAnalysis = true;
+ }
+ opts.AliasAnalysis = 0;
+ if (opts.OptimizationLevel > 0) {
+if (!noAliasAnalysis)
+ opts.AliasAnalysis = 1;
+ } else {
+if (aliasAnalysis)
+ opts.AliasAnalysis = 1;
+ }
for (auto *a : args.filtered(clang::driver::options::OPT_fpass_plugin_EQ))
[clang] [flang] [flang] Enable alias tags pass by default (PR #73111)
@@ -242,10 +242,24 @@ static void
parseCodeGenArgs(Fortran::frontend::CodeGenOptions ,
clang::driver::options::OPT_fno_loop_versioning, false))
opts.LoopVersioning = 1;
- opts.AliasAnalysis =
- args.hasFlag(clang::driver::options::OPT_falias_analysis,
- clang::driver::options::OPT_fno_alias_analysis,
- /*default=*/false);
+ bool aliasAnalysis = false;
+ bool noAliasAnalysis = false;
+ if (auto *arg =
+ args.getLastArg(clang::driver::options::OPT_falias_analysis,
+ clang::driver::options::OPT_fno_alias_analysis)) {
+if (arg->getOption().matches(clang::driver::options::OPT_falias_analysis))
+ aliasAnalysis = true;
+else
+ noAliasAnalysis = true;
+ }
+ opts.AliasAnalysis = 0;
+ if (opts.OptimizationLevel > 0) {
+if (!noAliasAnalysis)
+ opts.AliasAnalysis = 1;
tblah wrote:
It is a bitfield
https://github.com/llvm/llvm-project/blob/main/flang/include/flang/Frontend/CodeGenOptions.h#L37
https://github.com/llvm/llvm-project/pull/73111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
