Skip to content

[Fortran] Disabling error tests and tests not yet working with HLFIR #47

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 2 commits into from
Nov 14, 2023

Conversation

jeanPerier
Copy link
Contributor

Patch required before enabling HLFIR by default:
llvm/llvm-project#72090

Three batches of tests must be disabled.

  1. test using non constant length character elemental fuctions
  2. test using pointer assignments inside FORALL
  3. "error" test that previously "wrongly" passed (did not throw runtime errors as they should have)

For the first two batches, lowering without HFLIR handled a few cases , but did not fully handle the features correctly. With HLFIR, it was decided to make all cases TODO until the feature are handled correctly. They are not very common, hence I think the "feature regression" is acceptable for now compared to what HLFIR brings.

For, the last batch, the test suite is not set to run error checking tests (verifying a runtime error that is expected from the runtime given an invalid Fortran input), so the test suits considered a "pass" tests that where run with no error while invalid.

HLFIR with -O1 uses the runtime in cases where the previous lowering did things inline, the usage of the runtime allow catching these invalid program as expected, but since the test suit is not set up yet to deal with negative runtime tests, they must be disabled.

Patch required before enabling HLFIR by default:
llvm/llvm-project#72090

Three batches of tests must be disabled.
1. test using non constant length character elemental fuctions
2. test using pointer assignments inside FORALL
3. "error" test that previously "wrongly" pass (did not throw errors)

For the first two batches, lowering without HFLIR handled a few cases
, but did not fully handle the features correctly. With HLFIR, it was
decided to make all cases TODO until the feature are handled correctly.
They are not very common, hence I think the "feature regression" is
acceptable for now compared to what HLFIR brings.

For, the last batch, the test suite is not set to run error checking
tests (verifying a runtime error that is expected from the runtime
given an invalid Fortran input), so the test suits considered a "pass"
tests that where run with no error while invalid.

HLFIR with -O1 uses the runtime in cases where the previous lowering did
things inline, the usage of the runtime allow catching these invalid
program as expected, but since the test suit is not set up yet to deal
with negative runtime tests, they must be disabled.
@tblah
Copy link
Contributor

tblah commented Nov 13, 2023

I can see a lot of new (compared to the upstream HLFIR allow/deny lists) test failures running the testsuite with optimization enabled. It looks like something to do with TransposeAsElementalConversion. I will look at this today but I think this needs to be fixed first.

Other than that, I can see that your lists differ a bit from the upstream alow/deny list. If the allow/deny list is just out of date with bugfixes then that isn't a problem, its probably not worth fixing now, as I presume we can remove these after HLFIR is default.

@jeanPerier
Copy link
Contributor Author

I can see a lot of new (compared to the upstream HLFIR allow/deny lists) test failures running the testsuite with optimization enabled. It looks like something to do with TransposeAsElementalConversion. I will look at this today but I think this needs to be fixed first.

Thanks, yes I agree this seems like something that should be fixed before enabling HLFIR by default. I will hold on.

Other than that, I can see that your lists differ a bit from the upstream alow/deny list. If the allow/deny list is just out of date with bugfixes then that isn't a problem, its probably not worth fixing now, as I presume we can remove these after HLFIR is default.

Good point. I forgot about this list, it is a bit outdated (some HLFIR TODOs were implemented since making the HLFIR allow/deny list and the related tests are not disabled in this patch). I agree the HLFIR list should be updated or simply removed in a later patch.

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.

My comment earlier was a false alarm. I'm sorry for the noise. I had built my flang-new with a broken clang, causing my flang-new to segfault in one of the HLFIR passes. Rebuilding flang-new with a newer clang resolved the issue.

For future reference, the issue was reproducible with flang-new -flang-experimental-hlfir -O3 Fortran/gfortran/torture/execute/intrinsic_transpose.f90 -o /dev/null.

@jeanPerier jeanPerier merged commit 8352ecb into llvm:main Nov 14, 2023
@jeanPerier jeanPerier deleted the jpr-hlfir-todo branch November 14, 2023 11:02
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.

2 participants