Skip to content

[flang] Enable alias tags pass by default #73111

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
merged 8 commits into from
Nov 27, 2023
Merged

[flang] Enable alias tags pass by default #73111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6000017
[flang] Enable alias tags pass by default
tblah Sep 14, 2023
b776a35
Pass the whole MLIRToLLVMPassPipelineConfig to addFIRToLLVMPass()
tblah Nov 22, 2023
b5b0aa7
Rename variable and combine if condition
tblah Nov 22, 2023
5dbf3c2
fix formatting
tblah Nov 22, 2023
c2b7674
Refactor argument handling
tblah Nov 23, 2023
7d1aba6
Expand test to make behavior at different optimization levels explicit
tblah Nov 23, 2023
f50183c
Add another test to show that the alias analysis flags take precidenc…
tblah Nov 27, 2023
07601e8
Expand and move documentation comment£
tblah Nov 27, 2023
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
20 changes: 20 additions & 0 deletions clang/lib/Driver/ToolChains/Flang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
// only pass on the argument if it does not match that implied by the
// optimization level: so if optimization is requested, only forward
// -fno-alias-analysis. If optimization is not requested, only forward
// -falias-analysis.
Arg *optLevel =
Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4);
Comment on lines +151 to +152
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

if (aliasAnalysis) {
bool faliasAnalysis =
aliasAnalysis->getOption().matches(options::OPT_falias_analysis);
if (optLevel && !faliasAnalysis) {
CmdArgs.push_back("-fno-alias-analysis");
} else {
if (faliasAnalysis)
// 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,
Expand Down
6 changes: 3 additions & 3 deletions flang/include/flang/Tools/CLOptions.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,10 @@ inline void addDebugFoundationPass(mlir::PassManager &pm) {
}

inline void addFIRToLLVMPass(
mlir::PassManager &pm, llvm::OptimizationLevel optLevel = defaultOptLevel) {
mlir::PassManager &pm, const MLIRToLLVMPassPipelineConfig &config) {
fir::FIRToLLVMPassOptions options;
options.ignoreMissingTypeDescriptors = ignoreMissingTypeDescriptors;
options.applyTBAA = optLevel.isOptimizingForSpeed();
options.applyTBAA = config.AliasAnalysis;
options.forceUnifiedTBAATree = useOldAliasTags;
addPassConditionally(pm, disableFirToLlvmIr,
[&]() { return fir::createFIRToLLVMPass(options); });
Expand Down Expand Up @@ -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);
}

/// Create a pass pipeline for lowering from MLIR to LLVM IR
Expand Down
10 changes: 6 additions & 4 deletions flang/lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,12 @@ 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);
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);

for (auto *a : args.filtered(clang::driver::options::OPT_fpass_plugin_EQ))
opts.LLVMPassPlugins.push_back(a->getValue());
Expand Down
16 changes: 15 additions & 1 deletion flang/test/Driver/falias-analysis.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,26 @@
! 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
! RUN: %flang -c -emit-llvm -fno-alias-analysis -Ofast %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 -falias-analysis -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 -O3 %s -o - | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL
! RUN: %flang -fc1 -emit-llvm -O2 %s -o - | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL
! RUN: %flang -fc1 -emit-llvm -O1 %s -o - | FileCheck %s --check-prefix=CHECK-AA --check-prefix=CHECK-ALL

! RUN: %flang -fc1 -emit-llvm -O0 %s -o - | FileCheck %s --check-prefix=CHECK-NOAA --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 -fno-alias-analysis %s -o - | FileCheck %s --check-prefix=CHECK-NOAA --check-prefix=CHECK-ALL

subroutine simple(a)
integer, intent(inout) :: a(:)
Expand Down
2 changes: 2 additions & 0 deletions flang/test/Driver/mlir-pass-pipeline.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 9 additions & 13 deletions flang/test/Driver/optimization-remark.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions flang/test/Fir/basic-program.fir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions flang/tools/tco/tco.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down