[flang] Switch lowering to use the HLFIR step by default #72090
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Patch 3/3 of the transition step 1 described in https://discourse.llvm.org/t/rfc-enabling-the-hlfir-lowering-by-default/72778/7 This patch changes bbc and flang-new driver to use HLFIR lowering by default. `-hlfir=false` can be used with bbc and `-flang-deprecated-no-hlfir` with flang-new to get the previous default lowering behavior, but these options will only be available for a limited period of time. If any user needs these options to workaround bugs, they should open an issue against flang in llvm github repo so that the regression can be fixed in HLFIR.
jeanPerier
requested review from
sscalpone,
clementval,
banach-space,
tblah,
kiranchandramohan,
kkwli and
vzakhari
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: None (jeanPerier) ChangesPatch 3/3 of the transition step 1 described in This patch changes bbc and flang-new driver to use HLFIR lowering by default.
If any user needs these options to workaround bugs, they should open an issue against flang in llvm github repo so that the regression can be fixed in HLFIR. Full diff: https://github.com/llvm/llvm-project/pull/72090.diff 3 Files Affected:
diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def
index 7fb75f79bfc160a..503acdac869c7a0 100644
--- a/flang/include/flang/Lower/LoweringOptions.def
+++ b/flang/include/flang/Lower/LoweringOptions.def
@@ -27,14 +27,14 @@ ENUM_LOWERINGOPT(OptimizeTranspose, unsigned, 1, 1)
/// If true, enable polymorphic type lowering feature. Off by default.
ENUM_LOWERINGOPT(PolymorphicTypeImpl, unsigned, 1, 0)
-/// If true, lower to High level FIR before lowering to FIR.
-/// Off by default until fully ready.
-ENUM_LOWERINGOPT(LowerToHighLevelFIR, unsigned, 1, 0)
+/// If true, lower to High level FIR before lowering to FIR. On by default.
+ENUM_LOWERINGOPT(LowerToHighLevelFIR, unsigned, 1, 1)
/// If true, reverse PowerPC native vector element order.
ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0)
-/// If true, assume external names will be suffixed with an underscore. On by default.
+/// If true, assume external names will be suffixed with an underscore.
+/// On by default.
ENUM_LOWERINGOPT(Underscoring, unsigned, 1, 1)
#undef LOWERINGOPT
diff --git a/flang/test/HLFIR/hlfir-flags.f90 b/flang/test/HLFIR/hlfir-flags.f90
index 8ba9b21562a60ce..b383a79d12c27bf 100644
--- a/flang/test/HLFIR/hlfir-flags.f90
+++ b/flang/test/HLFIR/hlfir-flags.f90
@@ -1,20 +1,22 @@
-! Test -flang-experimental-hlfir (flang-new), -hlfir (bbc), -emit-hlfir, -emit-fir flags
+! Test -flang-deprecated-hlfir, -flang-experimental-hlfir (flang-new), and
+! -hlfir (bbc), -emit-hlfir, -emit-fir flags
! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck --check-prefix HLFIR --check-prefix ALL %s
! RUN: bbc -emit-hlfir -o - %s | FileCheck --check-prefix HLFIR --check-prefix ALL %s
-! RUN: %flang_fc1 -emit-hlfir -flang-experimental-hlfir -o - %s | FileCheck --check-prefix HLFIR --check-prefix ALL %s
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck --check-prefix HLFIR --check-prefix ALL %s
! RUN: bbc -emit-hlfir -hlfir -o - %s | FileCheck --check-prefix HLFIR --check-prefix ALL %s
-! RUN: %flang_fc1 -emit-fir -o - %s | FileCheck %s --check-prefix NO-HLFIR --check-prefix ALL
+! RUN: %flang_fc1 -emit-fir -o - %s | FileCheck --check-prefix FIR --check-prefix ALL %s
! RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -o - %s | FileCheck %s --check-prefix NO-HLFIR --check-prefix ALL
-! RUN: bbc -emit-fir -o - %s | FileCheck %s --check-prefix NO-HLFIR --check-prefix ALL
-! RUN: %flang_fc1 -emit-fir -flang-experimental-hlfir -o - %s | FileCheck --check-prefix FIR --check-prefix ALL %s
-! RUN: bbc -emit-fir -hlfir -o - %s | FileCheck --check-prefix FIR --check-prefix ALL %s
+! RUN: %flang_fc1 -emit-fir -flang-experimental-hlfir -o - %s | FileCheck %s --check-prefix FIR --check-prefix ALL
+! RUN: bbc -emit-fir -o - %s | FileCheck --check-prefix FIR --check-prefix ALL %s
+! RUN: bbc -emit-fir -hlfir=false -o - %s | FileCheck %s --check-prefix NO-HLFIR --check-prefix ALL
-! | Action | -flang-experimental-hlfir / -hlfir? | Result |
-! | =========== | =================================== | =============================== |
-! | -emit-hlfir | N | Outputs HLFIR |
-! | -emit-hlfir | Y | Outputs HLFIR |
-! | -emit-fir | N | Outputs FIR, using old lowering |
-! | -emit-fir | Y | Outputs FIR, lowering via HLFIR |
+! | Action | -flang-deprecated-no-hlfir | Result |
+! | | / -hlfir=false? | |
+! | =========== | =========================== | =============================== |
+! | -emit-hlfir | N | Outputs HLFIR |
+! | -emit-hlfir | Y | Outputs HLFIR |
+! | -emit-fir | N | Outputs FIR, lowering via HLFIR |
+! | -emit-fir | Y | Outputs FIR, using old lowering |
subroutine test(a, res)
real :: a(:), res
diff --git a/flang/tools/bbc/bbc.cpp b/flang/tools/bbc/bbc.cpp
index 378216bd1e51f8f..0a16ddb868875d7 100644
--- a/flang/tools/bbc/bbc.cpp
+++ b/flang/tools/bbc/bbc.cpp
@@ -193,7 +193,7 @@ static llvm::cl::opt<bool> enableNoPPCNativeVecElemOrder(
static llvm::cl::opt<bool> useHLFIR("hlfir",
llvm::cl::desc("Lower to high level FIR"),
- llvm::cl::init(false));
+ llvm::cl::init(true));
static llvm::cl::opt<bool> enableCUDA("fcuda",
llvm::cl::desc("enable CUDA Fortran"),
|
| ! RUN: bbc -emit-fir -o - %s | FileCheck %s --check-prefix NO-HLFIR --check-prefix ALL | ||
| ! RUN: %flang_fc1 -emit-fir -flang-experimental-hlfir -o - %s | FileCheck --check-prefix FIR --check-prefix ALL %s | ||
| ! RUN: bbc -emit-fir -hlfir -o - %s | FileCheck --check-prefix FIR --check-prefix ALL %s | ||
| ! RUN: %flang_fc1 -emit-fir -flang-experimental-hlfir -o - %s | FileCheck %s --check-prefix FIR --check-prefix ALL |
There was a problem hiding this comment.
I don't quite follow - what has changed here? It looks you still need a flag to enable this lowering in flang-new?
There was a problem hiding this comment.
This line did not change, it was just shuffle around. This still is not needed with this patch, see the test on (new) line 7: it has no switch and now uses --check-prefix FIR instead of --check-prefix NO-HLFIR
Since the -flang-experimental-hlfir switch is still available (and will be for a little time so that people can remove it from their workflow), so it is tested here that "it works" and that HFLIR is used.
There is one, see line 8 of the (new) file version. It is not added in this patch since this patch is not adding the switch. The core change of this patch is tested on (new) line 7 and 10: without any switches, bbc and flang_new output now checks for |
jeanPerier
added a commit
to jeanPerier/llvm-test-suite
that referenced
this pull request
Nov 13, 2023
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.
clementval
approved these changes
Nov 13, 2023
Thanks for pointing it out, I missed that. Great work! |
banach-space
approved these changes
Nov 13, 2023
jeanPerier
added a commit
to llvm/llvm-test-suite
that referenced
this pull request
Nov 14, 2023
…47) 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. maxlocval1 is also removed until more investigation is done (failing at -O3 on arm with HLFIR).
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
Patch 3/3 of the transition step 1 described in https://discourse.llvm.org/t/rfc-enabling-the-hlfir-lowering-by-default/72778/7 This patch changes bbc and flang-new driver to use HLFIR lowering by default. `-hlfir=false` can be used with bbc and `-flang-deprecated-no-hlfir` with flang-new to get the previous default lowering behavior, but these options will only be available for a limited period of time. If any user needs these options to workaround bugs, they should open an issue against flang in llvm github repo so that the regression can be fixed in HLFIR.
jsrob1n
pushed a commit
to jsrob1n/llvm-test-suite
that referenced
this pull request
Nov 28, 2023
…(#47) 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. maxlocval1 is also removed until more investigation is done (failing at -O3 on arm with HLFIR).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Patch 3/3 of the transition step 1 described in
https://discourse.llvm.org/t/rfc-enabling-the-hlfir-lowering-by-default/72778/7
This patch changes bbc and flang-new driver to use HLFIR lowering by default.
-hlfir=falsecan be used with bbc and-flang-deprecated-no-hlfirwith flang-new to get the previous default lowering behavior, but these options will only be available for a limited period of time.If any user needs these options to workaround bugs, they should open an issue against flang in llvm github repo so that the regression can be fixed in HLFIR.