Skip to content
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

[LoopRotate] Don't rotate loops when the minsize attribute is present #98815

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

aykevl
Copy link
Contributor

@aykevl aykevl commented Jul 14, 2024

The main use for this patch is LTO. It is not (yet?) possible to set the size level (-Os, -Oz) in the linker, which means loops are still rotated even if -Oz is specified on the command line. Therefore, look at the function attribute instead of only at the size level to determine whether to rotate loops for a given function.

For discussion, see: https://reviews.llvm.org/D119342

An older version of this patch was already approved at https://reviews.llvm.org/D119342 but I never got around to committing it. The code changed so I had to make some minor updates to this patch and in the meantime I also lost commit access because I wasn't really using it. So here is an updated patch.

The main use for this patch is LTO. It is not (yet?) possible to set the
size level (-Os, -Oz) in the linker, which means loops are still rotated
even if -Oz is specified on the command line. Therefore, look at the
function attribute instead of only at the size level to determine
whether to rotate loops for a given function.

For discussion, see: https://reviews.llvm.org/D119342
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 14, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Ayke (aykevl)

Changes

The main use for this patch is LTO. It is not (yet?) possible to set the size level (-Os, -Oz) in the linker, which means loops are still rotated even if -Oz is specified on the command line. Therefore, look at the function attribute instead of only at the size level to determine whether to rotate loops for a given function.

For discussion, see: https://reviews.llvm.org/D119342

An older version of this patch was already approved at https://reviews.llvm.org/D119342 but I never got around to committing it. The code changed so I had to make some minor updates to this patch and in the meantime I also lost commit access because I wasn't really using it. So here is an updated patch.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/LoopRotation.cpp (+5-4)
  • (added) llvm/test/Transforms/LoopRotate/minsize-disable.ll (+32)
diff --git a/llvm/lib/Transforms/Scalar/LoopRotation.cpp b/llvm/lib/Transforms/Scalar/LoopRotation.cpp
index 072859af4c5f9..acb79e94d087c 100644
--- a/llvm/lib/Transforms/Scalar/LoopRotation.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopRotation.cpp
@@ -64,10 +64,11 @@ PreservedAnalyses LoopRotatePass::run(Loop &L, LoopAnalysisManager &AM,
   // Vectorization requires loop-rotation. Use default threshold for loops the
   // user explicitly marked for vectorization, even when header duplication is
   // disabled.
-  int Threshold = EnableHeaderDuplication ||
-                          hasVectorizeTransformation(&L) == TM_ForcedByUser
-                      ? DefaultRotationThreshold
-                      : 0;
+  int Threshold =
+      (EnableHeaderDuplication && !L.getHeader()->getParent()->hasMinSize()) ||
+              hasVectorizeTransformation(&L) == TM_ForcedByUser
+          ? DefaultRotationThreshold
+          : 0;
   const DataLayout &DL = L.getHeader()->getDataLayout();
   const SimplifyQuery SQ = getBestSimplifyQuery(AR, DL);
 
diff --git a/llvm/test/Transforms/LoopRotate/minsize-disable.ll b/llvm/test/Transforms/LoopRotate/minsize-disable.ll
new file mode 100644
index 0000000000000..2db87b3ce8291
--- /dev/null
+++ b/llvm/test/Transforms/LoopRotate/minsize-disable.ll
@@ -0,0 +1,32 @@
+; REQUIRES: asserts
+; RUN: opt < %s -S -passes=loop-rotate -debug -debug-only=loop-rotate 2>&1 | FileCheck %s
+
+; Loop should not be rotated for functions with the minsize attribute.
+; This is mostly useful for LTO which doesn't (yet) understand -Oz.
+; CHECK: LoopRotation: NOT rotating - contains 2 instructions, which is more
+
+@e = global i32 10
+
+declare void @use(i32)
+
+; Function attrs: minsize optsize
+define void @test() #0 {
+entry:
+  %end = load i32, ptr @e
+  br label %loop
+
+loop:
+  %n.phi = phi i32 [ %n, %loop.fin ], [ 0, %entry ]
+  %cond = icmp eq i32 %n.phi, %end
+  br i1 %cond, label %exit, label %loop.fin
+
+loop.fin:
+  %n = add i32 %n.phi, 1
+  call void @use(i32 %n)
+  br label %loop
+
+exit:
+  ret void
+}
+
+attributes #0 = { minsize optsize }

@aykevl aykevl merged commit 91722a4 into llvm:main Jul 16, 2024
9 checks passed
@aykevl aykevl deleted the looprotate branch July 16, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants