[flang] Enable alias tags pass by default #73111

tblah · 2023-11-22T11:52:48Z

Enable by default for optimization levels higher than 0 (same behavior as clang).

For simplicity, only forward the flag to the frontend driver when it contradicts what is implied by the optimization level.

Since #72903 there are now no known performance regressions.

Original PR was #68597

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.

llvmbot · 2023-11-22T11:53:18Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-flang-driver

Author: Tom Eccles (tblah)

Changes

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.

Since #72903 there are now no known performance regressions.

Original PR was #68597

Full diff: https://github.com/llvm/llvm-project/pull/73111.diff

8 Files Affected:

(modified) clang/lib/Driver/ToolChains/Flang.cpp (+20)
(modified) flang/include/flang/Tools/CLOptions.inc (+4-4)
(modified) flang/lib/Frontend/CompilerInvocation.cpp (+18-4)
(modified) flang/test/Driver/falias-analysis.f90 (+4)
(modified) flang/test/Driver/mlir-pass-pipeline.f90 (+2)
(modified) flang/test/Driver/optimization-remark.f90 (+9-13)
(modified) flang/test/Fir/basic-program.fir (+4)
(modified) flang/tools/tco/tco.cpp (+1)

diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 8bdd920c3dcbb796..9382433b94dadfd4 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -142,6 +142,26 @@ void Flang::addCodegenOptions(const ArgList &Args,
   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 c452c023b4a80ce1..5a17385fb3dae87a 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -157,11 +157,11 @@ inline void addDebugFoundationPass(mlir::PassManager &pm) {
       [&]() { return fir::createAddDebugFoundationPass(); });
 }
 
-inline void addFIRToLLVMPass(
-    mlir::PassManager &pm, llvm::OptimizationLevel optLevel = defaultOptLevel) {
+inline void addFIRToLLVMPass(mlir::PassManager &pm,
+    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 cb4f2d6a6225205b..cfb1dd91ead30564 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -242,10 +242,24 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
                    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))
     opts.LLVMPassPlugins.push_back(a->getValue());
diff --git a/flang/test/Driver/falias-analysis.f90 b/flang/test/Driver/falias-analysis.f90
index f2c5dbde6d2c878c..1c74276974d47204 100644
--- a/flang/test/Driver/falias-analysis.f90
+++ b/flang/test/Driver/falias-analysis.f90
@@ -4,10 +4,14 @@
 ! 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 %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 -Ofast -fno-alias-analysis %s -o - | llvm-dis | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL
 
 ! RUN: %flang -fc1 -emit-llvm -falias-analysis %s -o - | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL
 ! RUN: %flang -fc1 -emit-llvm -falias-analysis -fno-alias-analysis %s -o - | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL
 ! RUN: %flang -fc1 -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL
+! RUN: %flang -fc1 -emit-llvm -O3 %s -o - | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL
+! RUN: %flang -fc1 -emit-llvm -O3 -fno-alias-analysis %s -o - | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL
 
 subroutine simple(a)
   integer, intent(inout) :: a(:)
diff --git a/flang/test/Driver/mlir-pass-pipeline.f90 b/flang/test/Driver/mlir-pass-pipeline.f90
index 7f92ec25bef98ec7..3d8c42f123e2eb06 100644
--- a/flang/test/Driver/mlir-pass-pipeline.f90
+++ b/flang/test/Driver/mlir-pass-pipeline.f90
@@ -51,6 +51,8 @@
 
 ! ALL-NEXT: 'func.func' Pipeline
 ! ALL-NEXT:   PolymorphicOpConversion
+! O2-NEXT:  AddAliasTags
+! O2-NEXT:  'func.func' Pipeline
 ! ALL-NEXT:   CFGConversion
 
 ! ALL-NEXT: SCFToControlFlow
diff --git a/flang/test/Driver/optimization-remark.f90 b/flang/test/Driver/optimization-remark.f90
index 13fc24346eac68b8..20ff9eb59a6702d6 100644
--- a/flang/test/Driver/optimization-remark.f90
+++ b/flang/test/Driver/optimization-remark.f90
@@ -41,28 +41,24 @@
 ! Once we start filtering, this is reduced to 1 one of the loop passes.
 
 ! PASS-REGEX-LOOP-ONLY-NOT:     optimization-remark.f90:77:7: remark: hoisting load [-Rpass=licm]
-! PASS-REGEX-LOOP-ONLY:         optimization-remark.f90:83:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! PASS-REGEX-LOOP-ONLY:         optimization-remark.f90:79:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
 
 ! MISSED-REGEX-LOOP-ONLY-NOT:   optimization-remark.f90:77:7: remark: failed to hoist load with loop-invariant address because load is conditionally executed [-Rpass-missed=licm]
-! MISSED-REGEX-LOOP-ONLY:       optimization-remark.f90:76:4: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! MISSED-REGEX-LOOP-ONLY:       optimization-remark.f90:72:4: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
 
 
-! ANALYSIS-REGEX-LOOP-ONLY:     optimization-remark.f90:79:7: remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
-! ANALYSIS-REGEX-LOOP-ONLY:     Unknown data dependence. Memory location is the same as accessed at optimization-remark.f90:78:7 [-Rpass-analysis=loop-vectorize]
+! ANALYSIS-REGEX-LOOP-ONLY:     optimization-remark.f90:73:7: remark: loop not vectorized: cannot identify array bounds [-Rpass-analysis=loop-vectorize]
 ! ANALYSIS-REGEX-LOOP-ONLY-NOT: remark: {{.*}}: IR instruction count changed from {{[0-9]+}} to {{[0-9]+}}; Delta: {{-?[0-9]+}} [-Rpass-analysis=size-info]
 
-! PASS:                         optimization-remark.f90:77:7: remark: hoisting load [-Rpass=licm]
-! PASS:                         optimization-remark.f90:83:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! PASS:                         optimization-remark.f90:79:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
 
-! MISSED:                       optimization-remark.f90:77:7: remark: failed to hoist load with loop-invariant address because load is conditionally executed [-Rpass-missed=licm]
-! MISSED:                       optimization-remark.f90:76:4: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
-! MISSED-NOT:                   optimization-remark.f90:79:7: remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
+! MISSED:                       optimization-remark.f90:73:7: remark: failed to move load with loop-invariant address because the loop may invalidate its value [-Rpass-missed=licm]
+! MISSED:                       optimization-remark.f90:72:4: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! MISSED-NOT:                   optimization-remark.f90:75:7: remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
 ! MISSED-NOT:                   Unknown data dependence. Memory location is the same as accessed at optimization-remark.f90:78:7 [-Rpass-analysis=loop-vectorize]
 
-! ANALYSIS:                     optimization-remark.f90:79:7: remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
-! ANALYSIS:                     Unknown data dependence. Memory location is the same as accessed at optimization-remark.f90:78:7 [-Rpass-analysis=loop-vectorize]
-! ANALYSIS:                     remark: {{.*}}: IR instruction count changed from {{[0-9]+}} to {{[0-9]+}}; Delta: {{-?[0-9]+}} [-Rpass-analysis=size-info]
-! ANALYSIS-NOT:                 optimization-remark.f90:77:7: remark: failed to hoist load with loop-invariant address because load is conditionally executed [-Rpass-missed=licm]
+! ANALYSIS:                     optimization-remark.f90:74:7: remark: loop not vectorized: unsafe dependent memory operations in loop.
+! ANALYSIS:                     remark: {{.*}} instructions in function [-Rpass-analysis=asm-printer]
 
 subroutine swap_real(a1, a2)
    implicit none
diff --git a/flang/test/Fir/basic-program.fir b/flang/test/Fir/basic-program.fir
index 0e82f7dfdedb447d..d8a9e74c318ce186 100644
--- a/flang/test/Fir/basic-program.fir
+++ b/flang/test/Fir/basic-program.fir
@@ -57,6 +57,10 @@ func.func @_QQmain() {
 
 // PASSES-NEXT: 'func.func' Pipeline
 // PASSES-NEXT:   PolymorphicOpConversion
+
+// PASSES-NEXT: AddAliasTags
+
+// PASSES-NEXT: 'func.func' Pipeline
 // PASSES-NEXT:   CFGConversion
 
 // PASSES-NEXT: SCFToControlFlow
diff --git a/flang/tools/tco/tco.cpp b/flang/tools/tco/tco.cpp
index 31d6bac142dc421b..a649535a39b74b31 100644
--- a/flang/tools/tco/tco.cpp
+++ b/flang/tools/tco/tco.cpp
@@ -120,6 +120,7 @@ compileFIR(const mlir::PassPipelineCLParser &passPipeline) {
       return mlir::failure();
   } else {
     MLIRToLLVMPassPipelineConfig config(llvm::OptimizationLevel::O2);
+    config.AliasAnalysis = true; // enabled when optimizing for speed
     if (codeGenLLVM) {
       // Run only CodeGen passes.
       fir::createDefaultFIRCodeGenPassPipeline(pm, config);

flang/include/flang/Tools/CLOptions.inc

flang/lib/Frontend/CompilerInvocation.cpp

Leporacanthicus

LGTM

banach-space

Nice!

Enable by default when optimizing for speed.

Please, can you be more specific and define what qualifies as "optimizing for speed"?

banach-space · 2023-11-22T21:08:17Z

flang/lib/Frontend/CompilerInvocation.cpp

+  bool aliasAnalysis = false;
+  bool noAliasAnalysis = false;


Why do we need two bools to model one thing? What's the logic that we trying to model here?

If -falias-analysis is specified then we should enable alias analysis even when it would not be enabled by the optimization level.

If -fno-alias-analysis is specified then we should not enable enable analysis even if it would be enabled by the optimization level.

This doesn't fit neatly into a single boolean, because we also need to support the state where both of these are false (indicating that we should follow the default behavior).

An alternative implementation would be a single boolean inside a std::option. Would that be clearer?

banach-space · 2023-11-22T21:11:15Z

clang/lib/Driver/ToolChains/Flang.cpp

+  Arg *optLevel =
+      Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);


What about other opt levels? Do we enable or disable alias analysis for these opt levels?

It would be disabled unless -falias-analysis is specified directly.

What I am trying to accomplish here is for the frontend driver to usually just do what you expect, without having to remember to use this option. But I want there to still be a separate flag available to override this default behavior.

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?

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.

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?

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

Some discussion regarding -Os is here.
https://discourse.llvm.org/t/code-size-optimization-flags-in-flang/69482

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);

clang/lib/Driver/ToolChains/Flang.cpp

github-actions · 2023-11-22T21:43:57Z

✅ With the latest revision this PR passed the C/C++ code formatter.

vzakhari

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?

vzakhari · 2023-11-22T22:03:07Z

clang/lib/Driver/ToolChains/Flang.cpp

+  Arg *optLevel =
+      Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);


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.

tblah · 2023-11-22T22:09:09Z

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.

banach-space

Makes sense, but the summary should document when exactly the alias analysis is enabled/disabled. And the relationship between -f{no}-alias-analysis and the optimisation flags.

Could you also add a note whether the implemented behaviour is consistent with Clang?

flang/lib/Frontend/CompilerInvocation.cpp

banach-space · 2023-11-23T10:53:18Z

clang/lib/Driver/ToolChains/Flang.cpp

+  Arg *optLevel =
+      Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);


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?

banach-space

Thanks for addressing my comments - this is looking really good now!

flang/test/Driver/falias-analysis.f90

clang/lib/Driver/ToolChains/Flang.cpp

…e over -O

kiranchandramohan

LGTM.

kiranchandramohan · 2023-11-27T13:58:04Z

clang/lib/Driver/ToolChains/Flang.cpp

+  Arg *optLevel =
+      Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);


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);

banach-space

I think that what's being proposed here is quite non-standard. In particular, what should happen here:

flang-new {A very long list of options copied from somewhere, including -fno-alias-analysis) -O3 file.f90

How is the user meant to know that they need to add -falias-analysis at the end to enable alias analysis? And in general, how are they supposed to know that -fno-alias-analysis overrides -O3?

If it was the case of "the last relevant option takes priority" (as is the case with most/all options) then that would be easy - identical logic would always apply.

This should be easy to fix if you use this instead of what's currently implemented (apologies for GitHub being unable to format this properly):

Args.getLastArg(options::OPT_falias_analysis,
                              options::OPT_fno_alias_analysis,
                               options::OPT_Ofast,
                               options::OPT_O,
                               options::OPT_O4);

(I'm skipping other changes that would also be required - hopefully this is clear enough). This should still give you all the flexibility that you need for testing and be less surprising for end users.

If you are in a rush to land this then this LGTM, but I would like the relationship between -O{1|2|3|4} and -f{no}-alias-analysis to be refined in a follow-up patch. Unless there's a good reason to avoid that? WDYT?

Sorry, I've only really realised this after @tblah updated falias-analysis.f90.

tblah · 2023-11-27T15:09:47Z

If you are in a rush to land this then this LGTM, but I would like the relationship between -O{1|2|3|4} and -f{no}-alias-analysis to be refined in a follow-up patch. Unless there's a good reason to avoid that? WDYT?

Thanks @banach-space, I will land this now and follow up later today.

The behavior you're commenting on was deliberate because to me, it feels wrong to enable alias analysis if there is an -fno-alias-analysis flag anywhere (and no -falias-analysis) as it it may not be obvious that -O implies -falias-analysis, and what -O does isn't documented anywhere.

But I don't feel strongly about it so I will follow up with a new patch later if you still feel that is worthwhile.

banach-space · 2023-11-27T16:20:59Z

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:

  PosFlag<SetTrue, [], [], "Pass alias information on to LLVM (overrides -O0 which disables alias analysis)">,
  NegFlag<SetFalse, [], [], "Do not pass alias information on to LLVM (overrides -O{1|2|3|4} which enable alias analysis)">>;

Btw, thanks for working on this - really great to see progress on this front! 🙏🏻

As requested by @branach-space on llvm#73111. This makes it clearer that -f[no-]alias-analysis will always override -O flags, no matter their ordering.

@vzakhari

This reverts commit caba031. Serious performance regressions were reported by @vzakhari llvm#58303 (comment) Fixing this doesn't look quick so I will revert for now.

@vzakhari

This reverts commit caba031. Serious performance regressions were reported by @vzakhari #58303 (comment) Fixing this doesn't look quick so I will revert for now.

Enable by default for optimization levels higher than 0 (same behavior as clang). For simplicity, only forward the flag to the frontend driver when it contradicts what is implied by the optimization level. This was first landed in llvm#73111 but was later reverted due to a performance regression. That regression was fixed by llvm#74065.

Enable by default for optimization levels higher than 0 (same behavior as clang). For simplicity, only forward the flag to the frontend driver when it contradicts what is implied by the optimization level. This was first landed in #73111 but was later reverted due to a performance regression. That regression was fixed by #74065.

[flang] Enable alias tags pass by default

6000017

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.

tblah requested review from kiranchandramohan, Renaud-K, vzakhari and victorkingi November 22, 2023 11:52

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' flang:driver flang Flang issues not falling into any other category labels Nov 22, 2023

kiranchandramohan requested a review from Leporacanthicus November 22, 2023 12:00

Leporacanthicus reviewed Nov 22, 2023

View reviewed changes

flang/include/flang/Tools/CLOptions.inc Outdated Show resolved Hide resolved

Leporacanthicus reviewed Nov 22, 2023

View reviewed changes

flang/lib/Frontend/CompilerInvocation.cpp Outdated Show resolved Hide resolved

Pass the whole MLIRToLLVMPassPipelineConfig to addFIRToLLVMPass()

b776a35

Leporacanthicus approved these changes Nov 22, 2023

View reviewed changes

banach-space reviewed Nov 22, 2023

View reviewed changes

Rename variable and combine if condition

b5b0aa7

fix formatting

5dbf3c2

vzakhari reviewed Nov 22, 2023

View reviewed changes

banach-space reviewed Nov 23, 2023

View reviewed changes

tblah added 2 commits November 23, 2023 11:55

Refactor argument handling

c2b7674

Expand test to make behavior at different optimization levels explicit

7d1aba6

tblah requested a review from banach-space November 27, 2023 10:22

banach-space reviewed Nov 27, 2023

View reviewed changes

flang/test/Driver/falias-analysis.f90 Show resolved Hide resolved

clang/lib/Driver/ToolChains/Flang.cpp Outdated Show resolved Hide resolved

tblah added 2 commits November 27, 2023 11:00

Add another test to show that the alias analysis flags take precidenc…

f50183c

…e over -O

Expand and move documentation comment£

07601e8

kiranchandramohan approved these changes Nov 27, 2023

View reviewed changes

banach-space reviewed Nov 27, 2023

View reviewed changes

tblah merged commit caba031 into llvm:main Nov 27, 2023
2 checks passed

tblah added a commit to tblah/llvm-project that referenced this pull request Nov 27, 2023

[flang] Update -falias-analysis help text

d65bd28

As requested by @branach-space on llvm#73111. This makes it clearer that -f[no-]alias-analysis will always override -O flags, no matter their ordering.

This was referenced Nov 27, 2023

[flang] Update -falias-analysis help text #73548

Closed

[Flang] Fix performance issue in 549.fotonik3d_r #58303

Open

tblah mentioned this pull request Dec 3, 2023

[flang] (Re-)Enable alias tags pass by default #74250

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flang] Enable alias tags pass by default #73111

[flang] Enable alias tags pass by default #73111

tblah commented Nov 22, 2023 •

edited

Loading

llvmbot commented Nov 22, 2023 •

edited

Loading

Leporacanthicus left a comment

banach-space left a comment

banach-space Nov 22, 2023

tblah Nov 22, 2023

banach-space Nov 22, 2023

tblah Nov 22, 2023

tblah Nov 22, 2023

vzakhari Nov 22, 2023

banach-space Nov 23, 2023

tblah Nov 23, 2023

kiranchandramohan Nov 27, 2023

kiranchandramohan Nov 27, 2023

github-actions bot commented Nov 22, 2023 •

edited

Loading

vzakhari left a comment

vzakhari Nov 22, 2023

tblah commented Nov 22, 2023

banach-space left a comment

banach-space Nov 23, 2023

banach-space left a comment

kiranchandramohan left a comment

kiranchandramohan Nov 27, 2023

banach-space left a comment

tblah commented Nov 27, 2023

banach-space commented Nov 27, 2023

		Arg *optLevel =
		Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);

[flang] Enable alias tags pass by default #73111

[flang] Enable alias tags pass by default #73111

Conversation

tblah commented Nov 22, 2023 • edited Loading

llvmbot commented Nov 22, 2023 • edited Loading

Leporacanthicus left a comment

Choose a reason for hiding this comment

banach-space left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 22, 2023 • edited Loading

vzakhari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tblah commented Nov 22, 2023

banach-space left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

banach-space left a comment

Choose a reason for hiding this comment

kiranchandramohan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

banach-space left a comment

Choose a reason for hiding this comment

tblah commented Nov 27, 2023

banach-space commented Nov 27, 2023

tblah commented Nov 22, 2023 •

edited

Loading

llvmbot commented Nov 22, 2023 •

edited

Loading

github-actions bot commented Nov 22, 2023 •

edited

Loading