Skip to content

[Clang][Driver] Add an option to control loop-interchange #125830

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.

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 1 commit into from
Feb 7, 2025

Conversation

sjoerdmeijer
Copy link
Collaborator

This introduces options -floop-interchange and -fno-loop-interchange to enable/disable the loop-interchange pass. This is part of the work that tries to get that pass enabled by default (#124911), where it was remarked that a user facing option to control this would be convenient to have. The option name is the same as GCC's.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Feb 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2025

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Sjoerd Meijer (sjoerdmeijer)

Changes

This introduces options -floop-interchange and -fno-loop-interchange to enable/disable the loop-interchange pass. This is part of the work that tries to get that pass enabled by default (#124911), where it was remarked that a user facing option to control this would be convenient to have. The option name is the same as GCC's.


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

3 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+3)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+10)
  • (modified) clang/test/Driver/clang_f_opts.c (+7)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index df705104d9ea314..3132f8946c6d3be 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4073,6 +4073,9 @@ defm assumptions : BoolFOption<"assumptions",
           "Disable codegen and compile-time checks for C++23's [[assume]] attribute">,
   PosFlag<SetTrue>>;
 
+def floop_interchange : Flag<["-"], "floop-interchange">, Group<f_Group>,
+  HelpText<"Enable the loop interchange pass">;
+def fno_loop_interchange: Flag<["-"], "fno-loop-interchange">, Group<f_Group>;
 def fvectorize : Flag<["-"], "fvectorize">, Group<f_Group>,
   HelpText<"Enable the loop vectorization passes">;
 def fno_vectorize : Flag<["-"], "fno-vectorize">, Group<f_Group>;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 33f08cf28feca18..757093c3d3a5cfa 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7619,6 +7619,16 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   Args.addOptOutFlag(CmdArgs, options::OPT_fgnu_inline_asm,
                      options::OPT_fno_gnu_inline_asm);
 
+  // Handle -floop-interchange
+  if (Arg *A = Args.getLastArg(options::OPT_floop_interchange,
+                               options::OPT_fno_loop_interchange)) {
+    CmdArgs.push_back("-mllvm");
+    if (A->getOption().matches(options::OPT_floop_interchange))
+      CmdArgs.push_back("-enable-loopinterchange=true");
+    else
+      CmdArgs.push_back("-enable-loopinterchange=false");
+  }
+
   // Enable vectorization per default according to the optimization level
   // selected. For optimization levels that want vectorization we use the alias
   // option to simplify the hasFlag logic.
diff --git a/clang/test/Driver/clang_f_opts.c b/clang/test/Driver/clang_f_opts.c
index 38f25898c955682..908d592928b01fc 100644
--- a/clang/test/Driver/clang_f_opts.c
+++ b/clang/test/Driver/clang_f_opts.c
@@ -156,6 +156,13 @@
 // CHECK-VECTORIZE: "-vectorize-loops"
 // CHECK-NO-VECTORIZE-NOT: "-vectorize-loops"
 
+// RUN: %clang -### -S -floop-interchange %s 2>&1 | FileCheck -check-prefix=CHECK-INTERCHANGE %s
+// RUN: %clang -### -S -fno-loop-interchange -floop-interchange %s 2>&1 | FileCheck -check-prefix=CHECK-INTERCHANGE %s
+// RUN: %clang -### -S -fno-loop-interchange %s 2>&1 | FileCheck -check-prefix=CHECK-NO-INTERCHANGE %s
+// RUN: %clang -### -S -floop-interchange -fno-loop-interchange %s 2>&1 | FileCheck -check-prefix=CHECK-NO-INTERCHANGE %s
+// CHECK-INTERCHANGE: "-mllvm" "-enable-loopinterchange=true"
+// CHECK-NO-INTERCHANGE: "-mllvm" "-enable-loopinterchange=false"
+
 // RUN: %clang -### -S -fslp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -fno-slp-vectorize -fslp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -fno-slp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SLP-VECTORIZE %s

@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2025

@llvm/pr-subscribers-clang-driver

Author: Sjoerd Meijer (sjoerdmeijer)

Changes

This introduces options -floop-interchange and -fno-loop-interchange to enable/disable the loop-interchange pass. This is part of the work that tries to get that pass enabled by default (#124911), where it was remarked that a user facing option to control this would be convenient to have. The option name is the same as GCC's.


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

3 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+3)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+10)
  • (modified) clang/test/Driver/clang_f_opts.c (+7)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index df705104d9ea31..3132f8946c6d3b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4073,6 +4073,9 @@ defm assumptions : BoolFOption<"assumptions",
           "Disable codegen and compile-time checks for C++23's [[assume]] attribute">,
   PosFlag<SetTrue>>;
 
+def floop_interchange : Flag<["-"], "floop-interchange">, Group<f_Group>,
+  HelpText<"Enable the loop interchange pass">;
+def fno_loop_interchange: Flag<["-"], "fno-loop-interchange">, Group<f_Group>;
 def fvectorize : Flag<["-"], "fvectorize">, Group<f_Group>,
   HelpText<"Enable the loop vectorization passes">;
 def fno_vectorize : Flag<["-"], "fno-vectorize">, Group<f_Group>;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 33f08cf28feca1..757093c3d3a5cf 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7619,6 +7619,16 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   Args.addOptOutFlag(CmdArgs, options::OPT_fgnu_inline_asm,
                      options::OPT_fno_gnu_inline_asm);
 
+  // Handle -floop-interchange
+  if (Arg *A = Args.getLastArg(options::OPT_floop_interchange,
+                               options::OPT_fno_loop_interchange)) {
+    CmdArgs.push_back("-mllvm");
+    if (A->getOption().matches(options::OPT_floop_interchange))
+      CmdArgs.push_back("-enable-loopinterchange=true");
+    else
+      CmdArgs.push_back("-enable-loopinterchange=false");
+  }
+
   // Enable vectorization per default according to the optimization level
   // selected. For optimization levels that want vectorization we use the alias
   // option to simplify the hasFlag logic.
diff --git a/clang/test/Driver/clang_f_opts.c b/clang/test/Driver/clang_f_opts.c
index 38f25898c95568..908d592928b01f 100644
--- a/clang/test/Driver/clang_f_opts.c
+++ b/clang/test/Driver/clang_f_opts.c
@@ -156,6 +156,13 @@
 // CHECK-VECTORIZE: "-vectorize-loops"
 // CHECK-NO-VECTORIZE-NOT: "-vectorize-loops"
 
+// RUN: %clang -### -S -floop-interchange %s 2>&1 | FileCheck -check-prefix=CHECK-INTERCHANGE %s
+// RUN: %clang -### -S -fno-loop-interchange -floop-interchange %s 2>&1 | FileCheck -check-prefix=CHECK-INTERCHANGE %s
+// RUN: %clang -### -S -fno-loop-interchange %s 2>&1 | FileCheck -check-prefix=CHECK-NO-INTERCHANGE %s
+// RUN: %clang -### -S -floop-interchange -fno-loop-interchange %s 2>&1 | FileCheck -check-prefix=CHECK-NO-INTERCHANGE %s
+// CHECK-INTERCHANGE: "-mllvm" "-enable-loopinterchange=true"
+// CHECK-NO-INTERCHANGE: "-mllvm" "-enable-loopinterchange=false"
+
 // RUN: %clang -### -S -fslp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -fno-slp-vectorize -fslp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-SLP-VECTORIZE %s
 // RUN: %clang -### -S -fno-slp-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SLP-VECTORIZE %s

@Meinersbur
Copy link
Member

The other optimzation pass options (unrolll, vectorize, ...) are implemented in PipelineTuningOptions and CodeGenOptions.def. Do it the same way?

@llvmbot llvmbot added clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Feb 6, 2025
@sjoerdmeijer
Copy link
Collaborator Author

The other optimzation pass options (unrolll, vectorize, ...) are implemented in PipelineTuningOptions and CodeGenOptions.def. Do it the same way?

Thanks for the review and suggestion @Meinersbur, that's now implemented in the latest revision.

@@ -316,6 +312,7 @@ PipelineTuningOptions::PipelineTuningOptions() {
LoopVectorization = true;
SLPVectorization = false;
LoopUnrolling = true;
LoopInterchange = false;
Copy link
Member

Choose a reason for hiding this comment

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

The default of LoopInterchange could be initialized with EnableLoopInterchange, as done by MergeFunctions = EnableMergeFunction.

Not important for me, but someone might want to keep -mllvm -enable-loopinterchange working, e.g. for non-Clang drivers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea, that's now fixed.

Copy link

github-actions bot commented Feb 6, 2025

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

@kasuga-fj
Copy link
Contributor

I'm not familiar with these parts, but the code looks good to me.

This is a bit off topic, but do you have any opinion on adding a pragma for interchange like other loop optimizations do? I think it can sometimes be useful if we can enable/disable the interchange for each loop, but I think there are a few things to consider if we introduce it, such as the behavior when the pragma is specified on a non-outermost loop. What do you think?

Copy link
Contributor

@madhur13490 madhur13490 left a comment

Choose a reason for hiding this comment

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

LGTM. Considering code-formatting issues will be fixed before committing.

Copy link
Member

@Meinersbur Meinersbur left a comment

Choose a reason for hiding this comment

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

LGTM

Dont forget clang-format (#125830 (comment))

This introduces options -floop-interchange and -fno-loop-interchange to
enable/disable the loop-interchange pass. This is part of the work that
tries to get that pass enabled by default (llvm#124911), where it was
remarked that a user facing option to control this would be convenient
to have. The option (name) is the same as GCC's.
@sjoerdmeijer sjoerdmeijer merged commit 612df14 into llvm:main Feb 7, 2025
8 checks passed
@sjoerdmeijer sjoerdmeijer deleted the clang-f-interchange branch February 7, 2025 10:31
@sjoerdmeijer
Copy link
Collaborator Author

This is a bit off topic, but do you have any opinion on adding a pragma for interchange like other loop optimizations do? I think it can sometimes be useful if we can enable/disable the interchange for each loop, but I think there are a few things to consider if we introduce it, such as the behavior when the pragma is specified on a non-outermost loop. What do you think?

Yeah, I see the use case for that, I think that makes sense. And OpenMP6 for example, introduces a directive for interchange. Please propose a patch if you're interested in pursuing this.

@Meinersbur
Copy link
Member

The OpenMP pragma conceptually applies interchange in the frontend, with syntactical requirements for the loops it applies to.

@kasuga-fj
Copy link
Contributor

Thanks for the comments. I'll try to work on it in the near future.

Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
This introduces options `-floop-interchange` and `-fno-loop-interchange`
to enable/disable the loop-interchange pass. This is part of the work
that tries to get that pass enabled by default (llvm#124911), where it was
remarked that a user facing option to control this would be convenient
to have. The option name is the same as GCC's.
kasuga-fj added a commit that referenced this pull request Mar 31, 2025
LoopInterchange has several heuristic functions to determine if
exchanging two loops is profitable or not. Whether or not to use each
heuristic and the order in which to use them were fixed, but #125830
allows them to be changed internally at will. This patch adds a new
option to control them via the compiler option.

The previous patch also added an option to prioritize the vectorization
heuristic. This patch also removes it to avoid conflicts between it and
the newly introduced one, e.g., both
`-loop-interchange-prioritize-vectorization=1` and
`-loop-interchange-profitabilities='cache,vectorization'` are specified.
kasuga-fj added a commit that referenced this pull request Apr 2, 2025
#133664)

LoopInterchange has several heuristic functions to determine if
exchanging two loops is profitable or not. Whether or not to use each
heuristic and the order in which to use them were fixed, but #125830
allows them to be changed internally at will. This patch adds a new
option to control them via the compiler option.

The previous patch also added an option to prioritize the vectorization
heuristic. This patch also removes it to avoid conflicts between it and
the newly introduced one, e.g., both
`-loop-interchange-prioritize-vectorization=1` and
`-loop-interchange-profitabilities='cache,vectorization'` are specified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants