Skip to content

[Fortran/gfortran] Disable tests only on AArch64, but enable them els… #187

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 7 commits into from
Dec 16, 2024

Conversation

tarunprabhu
Copy link
Contributor

…ewhere

Some tests fail only on AArch64, but pass on other platforms. Previously, these were disabled everywhere, but now they can be disabled only on AArch64 and enabled on other platforms. In addition, some tests that were failing only on AArch64 now pass, so they are enabled everywhere.

The static test configuration has also been updated to reflect these changes. Some bugs were exposed in the update script during this process. These have also been fixed.


@ylzsx,

Since these tests were preciously disabled on LoongArch, could you check that they pass there. If they do not, I will update the PR to disable them on LoongArch as well.

@ylzsx
Copy link
Contributor

ylzsx commented Dec 10, 2024

Thanks, I have tested this pr on a LoongArch machine. Two tests, findloc_8.f90 and pr91497.f90, fail to compile, with the same errors as on aarch64. Please disable these two tests on LoongArch64.

--- a/Fortran/gfortran/regression/override.yaml
+++ b/Fortran/gfortran/regression/override.yaml
@@ -25,7 +25,7 @@
 #     Assertion `Elt->getBitWidth() == EltVT.getSizeInBits() && "APInt size does not match type size!"' failed.
 #
 "findloc_8.f90":
-  disabled_on: ["aarch64-*-*"]
+  disabled_on: ["aarch64-*-*", "loongarch64-*-*"]
 
 # entry_23.f raises a segmentation fault at runtime, but only on AArch64.
 "entry_23.f":
@@ -35,7 +35,7 @@
 # error: 'kind=' argument must be a constant scalar integer whose value is a
 # supported kind for the intrinsic result type.
 "pr91497.f90":
-  disabled_on: ["aarch64-*-*"]
+  disabled_on: ["aarch64-*-*", "loongarch64-*-*"]

@jeanPerier
Copy link
Contributor

Thank you Tarun, this passes for me on X86-64.

Are they issues opened in llvm-project to cover the aarch64 failures?

If so, please add them in comment here, if not, it would be great to open some, even if the features are not meant to be supported there, it would be best to gracefully fail.

@jeanPerier jeanPerier requested a review from kkwli December 10, 2024 09:03
@jeanPerier
Copy link
Contributor

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.

Works for me on aarch64. Thank you Tarun

@tarunprabhu
Copy link
Contributor Author

Are they issues opened in llvm-project to cover the aarch64 failures?

If so, please add them in comment here, if not, it would be great to open some, even if the features are not meant to be supported there, it would be best to gracefully fail.

I was planning to start filing issues for some of these once I enabled some more tests, but now is as good a time as any, I suppose.

@tarunprabhu
Copy link
Contributor Author

Issues have been filed for these two tests:

entry_23.f: #119418
findloc_8.f90: #119420

@ylzsx
Copy link
Contributor

ylzsx commented Dec 11, 2024

Thanks, I have tested this pr on a LoongArch machine. Two tests, findloc_8.f90 and pr91497.f90, fail to compile, with the same errors as on aarch64. Please disable these two tests on LoongArch64.

@tarunprabhu It seems you have marked the wrong test case; the failing one is pr91497.f90, not entry_23.f on LoongArch.

@tarunprabhu
Copy link
Contributor Author

@tarunprabhu It seems you have marked the wrong test case; the failing one is pr91497.f90, not entry_23.f on LoongArch.

Ackh! Thanks for catching that. Fixed.

@kkwli
Copy link
Contributor

kkwli commented Dec 11, 2024

I tried the patch on powerpc64le. I got

FAIL: gfortran-regression-execute-regression__entry_23_f.test
NOEXE: gfortran-regression-execute-regression__findloc_8_f90.test
PASS: gfortran-regression-execute-regression__pr99210_f90.test
PASS: gfortran-regression-execute-regression__maxlocval_1_f90.test

Both entry_23.f and findloc_8.f90 fail with the same behavior as described in llvm/llvm-project#119418 and llvm/llvm-project#119420 respectively. However, when I try to disable these two test cases.

diff --git a/Fortran/gfortran/regression/override.yaml b/Fortran/gfortran/regression/override.yaml
index 88f312ce..7bed3ead 100644
--- a/Fortran/gfortran/regression/override.yaml
+++ b/Fortran/gfortran/regression/override.yaml
@@ -25,11 +25,11 @@
 #     Assertion `Elt->getBitWidth() == EltVT.getSizeInBits() && "APInt size does not match type size!"' failed.
 #
 "findloc_8.f90":
-  disabled_on: ["aarch64-*-*", "loongarch64-*-*"]
+  disabled_on: ["aarch64-*-*", "loongarch64-*-*", "powerpc64le-*-*"]
 
 # entry_23.f raises a segmentation fault at runtime, but only on AArch64.
 "entry_23.f":
-  disabled_on: ["aarch64-*-*"]
+  disabled_on: ["aarch64-*-*", "powerpc64le-*-*"]
 
 # pr91497.f90 fails to compile on some platforms with the following message:
 # error: 'kind=' argument must be a constant scalar integer whose value is a
diff --git a/Fortran/gfortran/regression/tests.cmake b/Fortran/gfortran/regression/tests.cmake
index 84762ad6..857dcc25 100644
--- a/Fortran/gfortran/regression/tests.cmake
+++ b/Fortran/gfortran/regression/tests.cmake
@@ -4701,7 +4701,7 @@ run;entry_12.f90;;;;
 run;entry_13.f90;;;;
 run;entry_14.f90;;;;
 run;entry_16.f90;;;;
-run;entry_23.f;;;;aarch64-*-* loongarch64-*-*
+run;entry_23.f;;;;aarch64-*-* loongarch64-*-* powerpc64le-*-*
 run;entry_26.f90;;-fno-f2c;;
 run;entry_27.f90;;-ff2c;;
 run;entry_3.f90;;;;
@@ -4806,7 +4806,7 @@ run;findloc_3.f90;;;;
 run;findloc_4.f90;;;;
 run;findloc_5.f90;;;;
 run;findloc_6.f90;;;;
-run;findloc_8.f90;;;;aarch64-*-* loongarch64-*-*
+run;findloc_8.f90;;;;aarch64-*-* loongarch64-*-* powerpc64le-*-*
 run;float_1.f90;;;;
 run;flush_1.f90;;;;
 run;fmt_bz_bn.f;;;;
@@ -6460,4 +6460,4 @@ run;zero_sized_3.f90;;;;
 run;zero_sized_4.f90;;;;
 run;zero_sized_5.f90;;;;
 run;zero_sized_8.f90;;;;
-run;zero_sized_9.f90;;;;
\ No newline at end of file
+run;zero_sized_9.f90;;;;

I got the same outcome.

FAIL: gfortran-regression-execute-regression__entry_23_f.test
NOEXE: gfortran-regression-execute-regression__findloc_8_f90.test
PASS: gfortran-regression-execute-regression__pr99210_f90.test
PASS: gfortran-regression-execute-regression__maxlocval_1_f90.test

@tarunprabhu
Copy link
Contributor Author

Thanks for checking, Kelvin. It's still odd that you are getting the NOEXE. That suggests that it is attempting to build findloc_8.f90 and failing. Do you mind checking what ${CMAKE_SYSTEM_PROCESSOR} and ${CMAKE_SYSTEM_NAME} return on your system? So far, that is the best hypothesis. We should try to get this resolved now.

@kkwli
Copy link
Contributor

kkwli commented Dec 11, 2024

Thanks for checking, Kelvin. It's still odd that you are getting the NOEXE. That suggests that it is attempting to build findloc_8.f90 and failing. Do you mind checking what ${CMAKE_SYSTEM_PROCESSOR} and ${CMAKE_SYSTEM_NAME} return on your system? So far, that is the best hypothesis. We should try to get this resolved now.

$ cat CMakeLists.txt 
message(STATUS "CMAKE_SYSTEM_PROCESSOR='${CMAKE_SYSTEM_PROCESSOR}'")
message(STATUS "CMAKE_SYSTEM_NAME='${CMAKE_SYSTEM_NAME}'")

Output on Linux:

...
-- CMAKE_SYSTEM_PROCESSOR='ppc64le'
-- CMAKE_SYSTEM_NAME='Linux'
...

Output on AIX:

-- CMAKE_SYSTEM_PROCESSOR='powerpc'
-- CMAKE_SYSTEM_NAME='AIX'

@ylzsx
Copy link
Contributor

ylzsx commented Dec 12, 2024

@tarunprabhu I sincerely apologize. After re-verifying, I found that entry_23.f also produces a similar error on LoongArch as it does on AArch64. Previously, running ninja check showed that the test passed, so I didn’t pay much attention. However, when I ran the test independently today, it also resulted in a segmentation fault.

Digging deeper into the cause, I discovered that during the CMake configuration, I had set the parameter TEST_SUITE_USE_PERF=ON, and with this setting, running ninja check passed without any issues. However, when TEST_SUITE_USE_PERF=OFF is set, the error occurs. TEST_SUITE_USE_PERF is defined https://github.com/llvm/llvm-test-suite/blob/main/tools/CMakeLists.txt#L23

I’m not sure whether it’s an issue with perf or timeit.sh. However, entry_23.f does indeed fail on LoongArch. I kindly ask you to mark it again. Once again, I sincerely apologize for my previous misunderstanding.

To summarize, it has now been found that a total of three test cases fail on LoongArch: findloc_8.f90 , pr91497.f90 and entry_23.f.

@tarunprabhu
Copy link
Contributor Author

That's quite alright, Zhaoxin. Happens to all of us :-)

It's curious that the test is reported as a pass with timeit.sh. I might take a look at it, but not right now.

@tarunprabhu
Copy link
Contributor Author

Thanks for checking, Kelvin. I'll take a closer look to try and understand what might be happening.

@tarunprabhu
Copy link
Contributor Author

I think one of the issues might have been that * is allowed as a wildcard match in the override files. But I don't think it can be used that way in cmake regexes. When parsing the DejaGNU directives from the test files, those are converted to .+ which is what cmake expects. But this was not done when parsing the override files. The latest commit fixes this.

@kkwli, could you check if the tests can be disabled on PowerPC with this change?

@kkwli
Copy link
Contributor

kkwli commented Dec 13, 2024

It works with using ppc64le (not powerpc64le). Thanks a lot.

diff --git a/Fortran/gfortran/regression/override.yaml b/Fortran/gfortran/regression/override.yaml
index 7b3e2e81..d7e5fa17 100644
--- a/Fortran/gfortran/regression/override.yaml
+++ b/Fortran/gfortran/regression/override.yaml
@@ -25,11 +25,11 @@
 #     Assertion `Elt->getBitWidth() == EltVT.getSizeInBits() && "APInt size does not match type size!"' failed.
 #
 "findloc_8.f90":
-  disabled_on: ["aarch64-*-*", "loongarch64-*-*"]
+  disabled_on: ["aarch64-*-*", "loongarch64-*-*", "ppc64le-*-*"]
 
 # entry_23.f raises a segmentation fault at runtime, on some platforms.
 "entry_23.f":
-  disabled_on: ["aarch64-*-*", "loongarch64-*-*"]
+  disabled_on: ["aarch64-*-*", "loongarch64-*-*", "ppc64le-*-*"]
 
 # pr91497.f90 fails to compile on some platforms with the following message:
 # error: 'kind=' argument must be a constant scalar integer whose value is a
diff --git a/Fortran/gfortran/regression/tests.cmake b/Fortran/gfortran/regression/tests.cmake
index 549182d5..e5cc6bf3 100644
--- a/Fortran/gfortran/regression/tests.cmake
+++ b/Fortran/gfortran/regression/tests.cmake
@@ -4701,7 +4701,7 @@ run;entry_12.f90;;;;
 run;entry_13.f90;;;;
 run;entry_14.f90;;;;
 run;entry_16.f90;;;;
-run;entry_23.f;;;;aarch64-.+-.+ loongarch64-.+-.+
+run;entry_23.f;;;;aarch64-.+-.+ loongarch64-.+-.+ ppc64le-.+-.+
 run;entry_26.f90;;-fno-f2c;;
 run;entry_27.f90;;-ff2c;;
 run;entry_3.f90;;;;
@@ -4806,7 +4806,7 @@ run;findloc_3.f90;;;;
 run;findloc_4.f90;;;;
 run;findloc_5.f90;;;;
 run;findloc_6.f90;;;;
-run;findloc_8.f90;;;;aarch64-.+-.+ loongarch64-.+-.+
+run;findloc_8.f90;;;;aarch64-.+-.+ loongarch64-.+-.+ ppc64le-.+-.+
 run;float_1.f90;;;;
 run;flush_1.f90;;;;
 run;fmt_bz_bn.f;;;;

No entry_23.f are findloc_8.f90 are found in the json file.

Copy link
Contributor

@kkwli kkwli left a comment

Choose a reason for hiding this comment

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

LG. Thanks.

…ewhere

Some tests fail only on AArch64, but pass on other platforms. Previously, these
were disabled everywhere, but now they can be disabled only on AArch64 and
enabled on other platforms. In addition, some tests that were failing only on
AArch64 now pass, so they are enabled everywhere.

The static test configuration has also been updated to reflect these changes.
Some bugs were exposed in the update script during this process. These have
also been fixed.
Tarun Prabhu and others added 4 commits December 16, 2024 08:18
…y handled

when parsing the target specifications from the test, but the same needs to be
done when parsing the override files since we allow the use of these wildcards
in those files as well.
@tarunprabhu tarunprabhu force-pushed the enable-tests-on-aarch64 branch from 21891e1 to 36fab2b Compare December 16, 2024 15:19
@tarunprabhu tarunprabhu merged commit 1645589 into llvm:main Dec 16, 2024
1 check passed
@tarunprabhu tarunprabhu deleted the enable-tests-on-aarch64 branch December 16, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants