Skip to content

Revert "[flang] Add -f[no-]unroll-loops flag (#122906)" #123779

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

Closed
wants to merge 1 commit into from

Conversation

mustartt
Copy link
Member

This commit causes an regression in ppc64-flang-aix and ppc64le-flang-rhel-clang. It only exposes that unrolling has no effect on powerpc target and the issue is tracked here #123668

This reverts commit 0195ec4.

@llvmbot 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 Jan 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 21, 2025

@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-flang-driver

Author: Henry Jiang (mustartt)

Changes

This commit causes an regression in ppc64-flang-aix and ppc64le-flang-rhel-clang. It only exposes that unrolling has no effect on powerpc target and the issue is tracked here #123668

This reverts commit 0195ec4.


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

8 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+2-2)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+3-3)
  • (modified) flang/include/flang/Frontend/CodeGenOptions.def (-1)
  • (modified) flang/lib/Frontend/CompilerInvocation.cpp (-4)
  • (modified) flang/lib/Frontend/FrontendActions.cpp (-2)
  • (removed) flang/test/Driver/funroll-loops.f90 (-5)
  • (removed) flang/test/HLFIR/unroll-loops.fir (-42)
  • (removed) flang/test/Integration/unroll-loops.f90 (-37)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 852051e772fc1c..4d1bd208754548 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4162,9 +4162,9 @@ def ftrap_function_EQ : Joined<["-"], "ftrap-function=">, Group<f_Group>,
   HelpText<"Issue call to specified function rather than a trap instruction">,
   MarshallingInfoString<CodeGenOpts<"TrapFuncName">>;
 def funroll_loops : Flag<["-"], "funroll-loops">, Group<f_Group>,
-  HelpText<"Turn on loop unroller">, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>;
+  HelpText<"Turn on loop unroller">, Visibility<[ClangOption, CC1Option]>;
 def fno_unroll_loops : Flag<["-"], "fno-unroll-loops">, Group<f_Group>,
-  HelpText<"Turn off loop unroller">, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>;
+  HelpText<"Turn off loop unroller">, Visibility<[ClangOption, CC1Option]>;
 def ffinite_loops: Flag<["-"],  "ffinite-loops">, Group<f_Group>,
   HelpText<"Assume all non-trivial loops are finite.">, Visibility<[ClangOption, CC1Option]>;
 def fno_finite_loops: Flag<["-"], "fno-finite-loops">, Group<f_Group>,
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 9c1fd28a3a8a26..8688a199018e84 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -156,9 +156,9 @@ void Flang::addCodegenOptions(const ArgList &Args,
                    options::OPT_fno_ppc_native_vec_elem_order,
                    options::OPT_fppc_native_vec_elem_order,
                    options::OPT_finit_global_zero,
-                   options::OPT_fno_init_global_zero, options::OPT_ftime_report,
-                   options::OPT_ftime_report_EQ, options::OPT_funroll_loops,
-                   options::OPT_fno_unroll_loops});
+                   options::OPT_fno_init_global_zero, 
+                   options::OPT_ftime_report,
+                   options::OPT_ftime_report_EQ});
 }
 
 void Flang::addPicOptions(const ArgList &Args, ArgStringList &CmdArgs) const {
diff --git a/flang/include/flang/Frontend/CodeGenOptions.def b/flang/include/flang/Frontend/CodeGenOptions.def
index deb8d1aede518b..9d03ec88a56b8a 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.def
+++ b/flang/include/flang/Frontend/CodeGenOptions.def
@@ -32,7 +32,6 @@ CODEGENOPT(PrepareForThinLTO , 1, 0) ///< Set when -flto=thin is enabled on the
                                      ///< compile step.
 CODEGENOPT(StackArrays, 1, 0) ///< -fstack-arrays (enable the stack-arrays pass)
 CODEGENOPT(LoopVersioning, 1, 0) ///< Enable loop versioning.
-CODEGENOPT(UnrollLoops, 1, 0) ///< Enable loop unrolling
 CODEGENOPT(AliasAnalysis, 1, 0) ///< Enable alias analysis pass
 
 CODEGENOPT(Underscoring, 1, 1)
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 3c6da4687f65d3..78d1199c19749b 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -246,10 +246,6 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
                    clang::driver::options::OPT_fno_loop_versioning, false))
     opts.LoopVersioning = 1;
 
-  opts.UnrollLoops = args.hasFlag(clang::driver::options::OPT_funroll_loops,
-                                  clang::driver::options::OPT_fno_unroll_loops,
-                                  (opts.OptimizationLevel > 1));
-
   opts.AliasAnalysis = opts.OptimizationLevel > 0;
 
   // -mframe-pointer=none/non-leaf/all option.
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index b0545a7ac2f99a..52a18d59c7cda5 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -1028,8 +1028,6 @@ void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
   si.registerCallbacks(pic, &mam);
   if (ci.isTimingEnabled())
     si.getTimePasses().setOutStream(ci.getTimingStreamLLVM());
-  pto.LoopUnrolling = opts.UnrollLoops;
-  pto.LoopInterleaving = opts.UnrollLoops;
   llvm::PassBuilder pb(targetMachine, pto, pgoOpt, &pic);
 
   // Attempt to load pass plugins and register their callbacks with PB.
diff --git a/flang/test/Driver/funroll-loops.f90 b/flang/test/Driver/funroll-loops.f90
deleted file mode 100644
index 5c1a07e7d5d12e..00000000000000
--- a/flang/test/Driver/funroll-loops.f90
+++ /dev/null
@@ -1,5 +0,0 @@
-! RUN: %flang -### -funroll-loops %s 2>&1 | FileCheck %s -check-prefix UNROLL
-! RUN: %flang -### -fno-unroll-loops %s 2>&1 | FileCheck %s -check-prefix NO-UNROLL
-
-! UNROLL: "-funroll-loops"
-! NO-UNROLL: "-fno-unroll-loops"
diff --git a/flang/test/HLFIR/unroll-loops.fir b/flang/test/HLFIR/unroll-loops.fir
deleted file mode 100644
index d8f820263ffd0d..00000000000000
--- a/flang/test/HLFIR/unroll-loops.fir
+++ /dev/null
@@ -1,42 +0,0 @@
-// RUN: %flang_fc1 -emit-llvm -O1 -funroll-loops -mllvm -force-vector-width=2 -o- %s | FileCheck %s --check-prefixes=CHECK,UNROLL
-// RUN: %flang_fc1 -emit-llvm -O2 -mllvm -force-vector-width=2 -o- %s | FileCheck %s --check-prefixes=CHECK,UNROLL
-// RUN: %flang_fc1 -emit-llvm -O1 -fno-unroll-loops -mllvm -force-vector-width=2 -o- %s | FileCheck %s --check-prefixes=CHECK,NO-UNROLL
-// RUN: %flang_fc1 -emit-llvm -O1 -mllvm -force-vector-width=2 -o- %s | FileCheck %s --check-prefixes=CHECK,NO-UNROLL
-
-// FIXME: https://github.com/llvm/llvm-project/issues/123668
-// XFAIL: powerpc64-target-arch
-
-// CHECK-LABEL: @unroll
-// CHECK-SAME: (ptr nocapture writeonly %[[ARG0:.*]])
-func.func @unroll(%arg0: !fir.ref<!fir.array<1000 x index>> {fir.bindc_name = "a"}) {
-  %scope = fir.dummy_scope : !fir.dscope
-  %c1000 = arith.constant 1000 : index
-  %shape = fir.shape %c1000 : (index) -> !fir.shape<1>
-  %a:2 = hlfir.declare %arg0(%shape) dummy_scope %scope {uniq_name = "unrollEa"} : (!fir.ref<!fir.array<1000xindex>>, !fir.shape<1>, !fir.dscope) -> (!fir.ref<!fir.array<1000 x index>>, !fir.ref<!fir.array<1000 x index>>)
-  %c1 = arith.constant 1 : index
-  fir.do_loop %arg1 = %c1 to %c1000 step %c1 {
-    // CHECK: br label %[[BLK:.*]]
-    // CHECK: [[BLK]]:
-    // CHECK-NEXT: %[[IND:.*]] = phi i64 [ 0, %{{.*}} ], [ %[[NIV:.*]], %[[BLK]] ]
-    // CHECK-NEXT: %[[VIND:.*]] = phi <2 x i64> [ <i64 1, i64 2>, %{{.*}} ], [ %[[NVIND:.*]], %[[BLK]] ]
-
-    // NO-UNROLL-NEXT: %[[GEP:.*]] = getelementptr i64, ptr %[[ARG0]], i64 %[[IND]]
-    // NO-UNROLL-NEXT: store <2 x i64> %[[VIND]], ptr %[[GEP]]
-    // NO-UNROLL-NEXT: %[[NIV:.*]] = add nuw i64 %{{.*}}, 2
-    // NO-UNROLL-NEXT: %[[NVIND]] = add <2 x i64> %[[VIND]], splat (i64 2)
-
-    // UNROLL-NEXT: %[[VIND1:.*]] = add <2 x i64> %[[VIND]], splat (i64 2)
-    // UNROLL-NEXT: %[[GEP0:.*]] = getelementptr i64, ptr %[[ARG0]], i64 %[[IND]]
-    // UNROLL-NEXT: %[[GEP1:.*]] = getelementptr i8, ptr %[[GEP0]], i64 16
-    // UNROLL-NEXT: store <2 x i64> %[[VIND]], ptr %[[GEP0]]
-    // UNROLL-NEXT: store <2 x i64> %[[VIND1]], ptr %[[GEP1]]
-    // UNROLL-NEXT: %[[NIV:.*]] = add nuw i64 %[[IND]], 4
-    // UNROLL-NEXT: %[[NVIND:.*]] = add <2 x i64> %[[VIND]], splat (i64 4)
-
-    // CHECK-NEXT: %[[EXIT:.*]] = icmp eq i64 %[[NIV]], 1000
-    // CHECK-NEXT: br i1 %[[EXIT]], label %{{.*}}, label %[[BLK]]
-    %ai = hlfir.designate %a#0 (%arg1)  : (!fir.ref<!fir.array<1000 x index>>, index) -> !fir.ref<index>
-    hlfir.assign %arg1 to %ai : index, !fir.ref<index>
-  }
-  return
-}
diff --git a/flang/test/Integration/unroll-loops.f90 b/flang/test/Integration/unroll-loops.f90
deleted file mode 100644
index 4a356c1ec5e9af..00000000000000
--- a/flang/test/Integration/unroll-loops.f90
+++ /dev/null
@@ -1,37 +0,0 @@
-! RUN: %flang_fc1 -emit-llvm -O1 -funroll-loops -mllvm -force-vector-width=2 -o- %s | FileCheck %s --check-prefixes=CHECK,UNROLL
-! RUN: %flang_fc1 -emit-llvm -O2 -mllvm -force-vector-width=2 -o- %s | FileCheck %s --check-prefixes=CHECK,UNROLL
-! RUN: %flang_fc1 -emit-llvm -O1 -fno-unroll-loops -mllvm -force-vector-width=2 -o- %s | FileCheck %s --check-prefixes=CHECK,NO-UNROLL
-! RUN: %flang_fc1 -emit-llvm -O1 -mllvm -force-vector-width=2 -o- %s | FileCheck %s --check-prefixes=CHECK,NO-UNROLL
-
-! FIXME: https://github.com/llvm/llvm-project/issues/123668
-! XFAIL: powerpc64-target-arch
-
-! CHECK-LABEL: @unroll
-! CHECK-SAME: (ptr nocapture writeonly %[[ARG0:.*]])
-subroutine unroll(a)
-  integer(kind=8), intent(out) :: a(1000)
-  integer(kind=8) :: i
-    ! CHECK: br label %[[BLK:.*]]
-    ! CHECK: [[BLK]]:
-    ! CHECK-NEXT: %[[IND:.*]] = phi i64 [ 0, %{{.*}} ], [ %[[NIV:.*]], %[[BLK]] ]
-    ! CHECK-NEXT: %[[VIND:.*]] = phi <2 x i64> [ <i64 1, i64 2>, %{{.*}} ], [ %[[NVIND:.*]], %[[BLK]] ]
-    !
-    ! NO-UNROLL-NEXT: %[[GEP:.*]] = getelementptr i64, ptr %[[ARG0]], i64 %[[IND]]
-    ! NO-UNROLL-NEXT: store <2 x i64> %[[VIND]], ptr %[[GEP]]
-    ! NO-UNROLL-NEXT: %[[NIV:.*]] = add nuw i64 %{{.*}}, 2
-    ! NO-UNROLL-NEXT: %[[NVIND]] = add <2 x i64> %[[VIND]], splat (i64 2)
-    !
-    ! UNROLL-NEXT: %[[VIND1:.*]] = add <2 x i64> %[[VIND]], splat (i64 2)
-    ! UNROLL-NEXT: %[[GEP0:.*]] = getelementptr i64, ptr %[[ARG0]], i64 %[[IND]]
-    ! UNROLL-NEXT: %[[GEP1:.*]] = getelementptr i8, ptr %[[GEP0]], i64 16
-    ! UNROLL-NEXT: store <2 x i64> %[[VIND]], ptr %[[GEP0]]
-    ! UNROLL-NEXT: store <2 x i64> %[[VIND1]], ptr %[[GEP1]]
-    ! UNROLL-NEXT: %[[NIV:.*]] = add nuw i64 %[[IND]], 4
-    ! UNROLL-NEXT: %[[NVIND:.*]] = add <2 x i64> %[[VIND]], splat (i64 4)
-    !
-    ! CHECK-NEXT: %[[EXIT:.*]] = icmp eq i64 %[[NIV]], 1000
-    ! CHECK-NEXT: br i1 %[[EXIT]], label %{{.*}}, label %[[BLK]]
-  do i=1,1000
-    a(i) = i
-  end do
-end subroutine

@mustartt mustartt closed this Jan 21, 2025
Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff a79098bc726e8de85d3ed0050de5395015bca031 2c651ade2a8ae018588f6012b7015f7fab8af4e3 --extensions cpp -- clang/lib/Driver/ToolChains/Flang.cpp flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/FrontendActions.cpp
View the diff from clang-format here.
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 8688a19901..c46e8222a9 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -156,8 +156,7 @@ void Flang::addCodegenOptions(const ArgList &Args,
                    options::OPT_fno_ppc_native_vec_elem_order,
                    options::OPT_fppc_native_vec_elem_order,
                    options::OPT_finit_global_zero,
-                   options::OPT_fno_init_global_zero, 
-                   options::OPT_ftime_report,
+                   options::OPT_fno_init_global_zero, options::OPT_ftime_report,
                    options::OPT_ftime_report_EQ});
 }
 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category flang:driver flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants