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

[flang][NFC] Restrict -funroll-loops tests to known working targets #124594

Merged
merged 2 commits into from
Jan 28, 2025

Conversation

DavidTruby
Copy link
Member

If -funroll-loops tests are not restricted to specific targets the tests may behave differently based on the host platform. This patch restricts the tests to aarch64 and x86_64, and removes the PowerPC XFAIL.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Jan 27, 2025
@DavidTruby
Copy link
Member Author

DavidTruby commented Jan 27, 2025

@mustartt I've tried again to fix the ppc failures here. I think it should work this time, as well as resolving the sparc failure reported by @rorth

@llvmbot
Copy link
Member

llvmbot commented Jan 27, 2025

@llvm/pr-subscribers-flang-fir-hlfir

Author: David Truby (DavidTruby)

Changes

If -funroll-loops tests are not restricted to specific targets the tests may behave differently based on the host platform. This patch restricts the tests to aarch64 and x86_64, and removes the PowerPC XFAIL.


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

2 Files Affected:

  • (modified) flang/test/Integration/unroll-loops.f90 (+12-8)
  • (renamed) flang/test/Lower/HLFIR/unroll-loops.fir (+10-6)
diff --git a/flang/test/Integration/unroll-loops.f90 b/flang/test/Integration/unroll-loops.f90
index 4b4a3945028814..30c04203ac0cbd 100644
--- a/flang/test/Integration/unroll-loops.f90
+++ b/flang/test/Integration/unroll-loops.f90
@@ -1,11 +1,15 @@
-! 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: target=powerpc64{{.*}}
-
+! DEFINE: %{triple} =
+! DEFINE: %{check-unroll} = %flang_fc1 -emit-llvm -O1 -funroll-loops -mllvm -force-vector-width=2 -triple %{triple} -o- %s | FileCheck %s --check-prefixes=CHECK,UNROLL
+! DEFINE: %{check-nounroll} = %flang_fc1 -emit-llvm -O1 -mllvm -force-vector-width=2 -triple %{triple} -o- %s | FileCheck %s --check-prefixes=CHECK,NO-UNROLL
+!
+! REDEFINE: %{triple} = aarch64-unknown-linux-gnu
+! RUN: %if aarch64-registered-target %{ %{check-unroll} %}
+! RUN: %if aarch64-registered-target %{ %{check-nounroll} %}
+!
+! REDEFINE: %{triple} = x86_64-unknown-linux-gnu
+! RUN: %if x86-registered-target %{ %{check-unroll} %}
+! RUN: %if x86-registered-target %{ %{check-nounroll} %}
+!
 ! CHECK-LABEL: @unroll
 ! CHECK-SAME: (ptr nocapture writeonly %[[ARG0:.*]])
 subroutine unroll(a)
diff --git a/flang/test/HLFIR/unroll-loops.fir b/flang/test/Lower/HLFIR/unroll-loops.fir
similarity index 73%
rename from flang/test/HLFIR/unroll-loops.fir
rename to flang/test/Lower/HLFIR/unroll-loops.fir
index 4494cfa570dd7b..42a236721e4c15 100644
--- a/flang/test/HLFIR/unroll-loops.fir
+++ b/flang/test/Lower/HLFIR/unroll-loops.fir
@@ -1,10 +1,14 @@
-// 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
+// DEFINE: %{triple} =
+// DEFINE: %{check-unroll} = %flang_fc1 -emit-llvm -O1 -funroll-loops -mllvm -force-vector-width=2 -triple %{triple} -o- %s | FileCheck %s --check-prefixes=CHECK,UNROLL
+// DEFINE: %{check-nounroll} = %flang_fc1 -emit-llvm -O1 -mllvm -force-vector-width=2 -triple %{triple} -o- %s | FileCheck %s --check-prefixes=CHECK,NO-UNROLL
 
-// FIXME: https://github.com/llvm/llvm-project/issues/123668
-// XFAIL: target=powerpc64{{.*}}
+// REDEFINE: %{triple} = aarch64-unknown-linux-gnu
+// RUN: %if aarch64-registered-target %{ %{check-unroll} %}
+// RUN: %if aarch64-registered-target %{ %{check-nounroll} %}
+
+// REDEFINE: %{triple} = x86_64-unknown-linux-gnu
+// RUN: %if x86-registered-target %{ %{check-unroll} %}
+// RUN: %if x86-registered-target %{ %{check-nounroll} %}
 
 // CHECK-LABEL: @unroll
 // CHECK-SAME: (ptr nocapture writeonly %[[ARG0:.*]])

Copy link
Contributor

@mustartt mustartt left a comment

Choose a reason for hiding this comment

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

Thank you for updating the test case. We don't have non PPC target enabled in the buildbut. I will add our PPC expected test case soon.

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

Thanks

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

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

Might want to keep the PPC issue links but otherwise LGTM.

! 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds like this issue may get fixed very soon, but I would still leave the link in here just in case that does not happen.


! DEFINE: %{triple} =
! DEFINE: %{check-unroll} = %flang_fc1 -emit-llvm -O1 -funroll-loops -mllvm -force-vector-width=2 -triple %{triple} -o- %s | FileCheck %s --check-prefixes=CHECK,UNROLL
! DEFINE: %{check-nounroll} = %flang_fc1 -emit-llvm -O1 -mllvm -force-vector-width=2 -triple %{triple} -o- %s | FileCheck %s --check-prefixes=CHECK,NO-UNROLL
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like every day I learn a new lit feature.

If -funroll-loops tests are not restricted to specific targets the tests
may behave differently based on the host platform. This patch restricts
the tests to aarch64 and x86_64, and removes the PowerPC XFAIL.
@DavidTruby DavidTruby merged commit 6cb71d7 into llvm:main Jan 28, 2025
8 checks passed
@DavidTruby DavidTruby deleted the funroll branch January 28, 2025 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants