[flang]Add new intrinsic function backtrace and complete the TODO of abort #117603
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
flang:runtime
flang
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-fir-hlfir Author: 执着 (dty2) ChangesHey guys, I found that Flang's built-in ABORT function is incomplete when I was using it. Compared with gfortran's ABORT (which can both abort and print out a backtrace), flang's ABORT implementation lacks the function of printing out a backtrace. This feature is essential for debugging and understanding the call stack at the failure point. To solve this problem, I completed the "// TODO:" of the abort function, and then implemented an additional built-in function BACKTRACE for flang. After a brief reading of the relevant source code, I used backtrace and backtrace_symbols in "execinfo.h" to quickly implement this. But since I used the above two functions directly, my implementation is slightly different from gfortran's implementation (in the output, the function call stack before main is additionally output, and the function line number is missing). In addition, since I used the above two functions, I did not need to add -g to embed debug information into the ELF file, but needed -rdynamic to ensure that the symbols are added to the dynamic symbol table (so that the function name will be printed out). Here is a comparison of the output between gfortran 's backtrace and my implementation: my impelmention output: test program is: I am well aware of the importance of line numbers, so I am now working on implementing line numbers (by parsing DWARF information) and supporting cross-platform (Windows) support. Full diff: https://github.com/llvm/llvm-project/pull/117603.diff 8 Files Affected:
diff --git a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
index e83d1a42e34133..7f4b9ebf1d1c21 100644
--- a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
+++ b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
@@ -196,6 +196,7 @@ struct IntrinsicLibrary {
fir::ExtendedValue genAssociated(mlir::Type,
llvm::ArrayRef<fir::ExtendedValue>);
mlir::Value genAtand(mlir::Type, llvm::ArrayRef<mlir::Value>);
+ void genBacktrace(llvm::ArrayRef<fir::ExtendedValue>);
fir::ExtendedValue genBesselJn(mlir::Type,
llvm::ArrayRef<fir::ExtendedValue>);
fir::ExtendedValue genBesselYn(mlir::Type,
diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Stop.h b/flang/include/flang/Optimizer/Builder/Runtime/Stop.h
index 6f764badf6f3a8..be73cffff021e3 100644
--- a/flang/include/flang/Optimizer/Builder/Runtime/Stop.h
+++ b/flang/include/flang/Optimizer/Builder/Runtime/Stop.h
@@ -30,6 +30,9 @@ void genExit(fir::FirOpBuilder &, mlir::Location, mlir::Value status);
/// Generate call to ABORT intrinsic runtime routine.
void genAbort(fir::FirOpBuilder &, mlir::Location);
+/// Generate call to BACKTRACE intrinsic runtime routine.
+void genBacktrace(fir::FirOpBuilder &builder, mlir::Location loc);
+
/// Generate call to crash the program with an error message when detecting
/// an invalid situation at runtime.
void genReportFatalUserError(fir::FirOpBuilder &, mlir::Location,
diff --git a/flang/include/flang/Runtime/stop.h b/flang/include/flang/Runtime/stop.h
index f7c4ffe7403e8e..d442f72bfe1fa4 100644
--- a/flang/include/flang/Runtime/stop.h
+++ b/flang/include/flang/Runtime/stop.h
@@ -29,6 +29,7 @@ NORETURN void RTNAME(ProgramEndStatement)(NO_ARGUMENTS);
// Extensions
NORETURN void RTNAME(Exit)(int status DEFAULT_VALUE(EXIT_SUCCESS));
NORETURN void RTNAME(Abort)(NO_ARGUMENTS);
+void RTNAME(Backtrace)(NO_ARGUMENTS);
// Crash with an error message when the program dynamically violates a Fortran
// constraint.
diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index 1e27c0ae4216c5..599a7d0124b800 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -1333,6 +1333,7 @@ static const IntrinsicInterface intrinsicSubroutine[]{
{"stat", AnyInt, Rank::scalar, Optionality::optional,
common::Intent::Out}},
{}, Rank::elemental, IntrinsicClass::atomicSubroutine},
+ {"backtrace", {}, {}, Rank::elemental, IntrinsicClass::pureSubroutine},
{"co_broadcast",
{{"a", AnyData, Rank::anyOrAssumedRank, Optionality::required,
common::Intent::InOut},
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index a2b327f45c6939..c748c6583a5ce9 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -150,6 +150,7 @@ static constexpr IntrinsicHandler handlers[]{
{"atan2pi", &I::genAtanpi},
{"atand", &I::genAtand},
{"atanpi", &I::genAtanpi},
+ {"backtrace", &I::genBacktrace},
{"bessel_jn",
&I::genBesselJn,
{{{"n1", asValue}, {"n2", asValue}, {"x", asValue}}},
@@ -2681,6 +2682,12 @@ IntrinsicLibrary::genBesselJn(mlir::Type resultType,
}
}
+// Backtrace
+void IntrinsicLibrary::genBacktrace(llvm::ArrayRef<fir::ExtendedValue> args) {
+ assert(args.size() == 0);
+ fir::runtime::genBacktrace(builder, loc);
+}
+
// BESSEL_YN
fir::ExtendedValue
IntrinsicLibrary::genBesselYn(mlir::Type resultType,
diff --git a/flang/lib/Optimizer/Builder/Runtime/Stop.cpp b/flang/lib/Optimizer/Builder/Runtime/Stop.cpp
index 411181cc6dd1ca..541e5f3b5d11a8 100644
--- a/flang/lib/Optimizer/Builder/Runtime/Stop.cpp
+++ b/flang/lib/Optimizer/Builder/Runtime/Stop.cpp
@@ -28,6 +28,13 @@ void fir::runtime::genAbort(fir::FirOpBuilder &builder, mlir::Location loc) {
builder.create<fir::CallOp>(loc, abortFunc, std::nullopt);
}
+void fir::runtime::genBacktrace(fir::FirOpBuilder &builder,
+ mlir::Location loc) {
+ mlir::func::FuncOp backtraceFunc =
+ fir::runtime::getRuntimeFunc<mkRTKey(Backtrace)>(loc, builder);
+ builder.create<fir::CallOp>(loc, backtraceFunc, std::nullopt);
+}
+
void fir::runtime::genReportFatalUserError(fir::FirOpBuilder &builder,
mlir::Location loc,
llvm::StringRef message) {
diff --git a/flang/runtime/stop.cpp b/flang/runtime/stop.cpp
index cfb36b40840200..57209dc37befa7 100644
--- a/flang/runtime/stop.cpp
+++ b/flang/runtime/stop.cpp
@@ -15,6 +15,7 @@
#include <cfenv>
#include <cstdio>
#include <cstdlib>
+#include <execinfo.h>
extern "C" {
@@ -152,11 +153,29 @@ void RTNAME(PauseStatementText)(const char *code, std::size_t length) {
std::exit(status);
}
+static void PrintBacktrace() {
+ // TODO: Need to parse DWARF information to print function line numbers
+ const int MAX_CALL_STACK = 999;
+ void *buffer[MAX_CALL_STACK];
+ int nptrs = backtrace(buffer, MAX_CALL_STACK);
+ char **symbols = backtrace_symbols(buffer, nptrs);
+ if (symbols == nullptr) {
+ Fortran::runtime::Terminator{}.Crash("no symbols");
+ std::exit(EXIT_FAILURE);
+ }
+ for (int i = 0; i < nptrs; i++) {
+ Fortran::runtime::Terminator{}.PrintCrashArgs("#%d %s\n", i, symbols[i]);
+ }
+ free(symbols);
+}
+
[[noreturn]] void RTNAME(Abort)() {
- // TODO: Add backtrace call, unless with `-fno-backtrace`.
+ PrintBacktrace();
std::abort();
}
+void RTNAME(Backtrace)() { PrintBacktrace(); }
+
[[noreturn]] void RTNAME(ReportFatalUserError)(
const char *message, const char *source, int line) {
Fortran::runtime::Terminator{source, line}.Crash(message);
diff --git a/flang/test/Lower/Intrinsics/backtrace.f90 b/flang/test/Lower/Intrinsics/backtrace.f90
new file mode 100644
index 00000000000000..9d5e7b4965baff
--- /dev/null
+++ b/flang/test/Lower/Intrinsics/backtrace.f90
@@ -0,0 +1,10 @@
+! RUN: bbc -emit-fir %s -o - | FileCheck %s
+
+! CHECK-LABEL: func.func @_QPbacktrace_test() {
+! CHECK: %[[VAL_0:.*]] = fir.call @_FortranABacktrace() {{.*}}: () -> none
+! CHECK: return
+! CHECK: }
+
+subroutine backtrace_test()
+ call backtrace
+end subroutine
|
|
@kparzysz Could you please review the latest changes? I've addressed all the feedback from the previous review. Thank you! And I don't have merge permission. I hope you can merge the code if there is no problem after checking. Thank you very much. |
|
@kparzysz Thank you for your patience in guiding my code. Please review the latest changes again to see if they meet the merge criteria. I will try my best to implement the -fno-backtrace option. If the implementation is successful, I will submit another PR. And I will continue to do my part for the llvm and flang communities. |
|
Looks good to me. |
|
Oh, thanks, Can you merge this code? I don't have merge permissions |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
It looks like you need to run clang-format. Please make sure that the pre-merge checks pass before merging.
Other than that, LGTM. Thanks for seeing this through.
|
@dty2 Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/152/builds/710 Here is the relevant piece of the build log for the reference |
…TODO of abort" (#117990) Reverts #117603 due to failed buildbot https://lab.llvm.org/buildbot/#/builders/152/builds/710 The important bit of the log was ``` FAILED: CMakeFiles/FortranRuntime.dir/stop.cpp.o ccache /usr/bin/g++ -DFLANG_LITTLE_ENDIAN=1 -DGTEST_HAS_RTTI=0 -DRT_USE_LIBCUDACXX=1 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/buildbot/worker/as-builder-7/ramdisk/flang-runtime-cuda-gcc/llvm-project/flang/runtime/../include -I/home/buildbot/worker/as-builder-7/ramdisk/flang-runtime-cuda-gcc/build -I/home/buildbot/worker/third-party/nv/cccl/libcudacxx/include -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -Wimplicit-fallthrough -Wno-nonnull -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-lto -O3 -DNDEBUG -U_GLIBCXX_ASSERTIONS -U_LIBCPP_ENABLE_ASSERTIONS -UNDEBUG -std=c++17 -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -MD -MT CMakeFiles/FortranRuntime.dir/stop.cpp.o -MF CMakeFiles/FortranRuntime.dir/stop.cpp.o.d -o CMakeFiles/FortranRuntime.dir/stop.cpp.o -c /home/buildbot/worker/as-builder-7/ramdisk/flang-runtime-cuda-gcc/llvm-project/flang/runtime/stop.cpp /home/buildbot/worker/as-builder-7/ramdisk/flang-runtime-cuda-gcc/llvm-project/flang/runtime/stop.cpp:19:10: fatal error: llvm/Config/config.h: No such file or directory 19 | #include "llvm/Config/config.h" | ^~~~~~~~~~~~~~~~~~~~~~ compilation terminated. ``` CC @dty2
|
I reverted due to the failed buildbot. The buildbot failed because the LLVM configuration header was missing. The buildbot had a separate step for building the runtime so I wonder if it built only that and nothing in It looks like there is already a Thanks for all of your work so far :) don't hesitate to reach out if you have any questions. |
| @@ -1336,6 +1336,7 @@ static const IntrinsicInterface intrinsicSubroutine[]{ | |||
| {"stat", AnyInt, Rank::scalar, Optionality::optional, | |||
| common::Intent::Out}}, | |||
| {}, Rank::elemental, IntrinsicClass::atomicSubroutine}, | |||
| {"backtrace", {}, {}, Rank::elemental, IntrinsicClass::pureSubroutine}, | |||
There was a problem hiding this comment.
I am late in the review, but since this has been reverted, I do not think "elemental" is correct here, and I would also debate the pure aspect since this is doing IO on the standard output which is forbidden for pure subroutine.
In fact, I am not quite sure that the front-end needs to know about backtrace since there are type resolution or argument requiring explicit interface.
It may just be simpler to deal with it like a user procedure for which the runtime provides an implementation (if the user has not). See FDATE implementation for instance.
Please also document the extension support in docs/Intrinsics.md.
Thanks for adding this, it is a great feature.
There was a problem hiding this comment.
Oh! I understand what you said. I think you are right. But in this case, it seems to me that the built-in functions such as Abort and Exit need to be rewritten, because they can be regarded as user processes like backtrace. (If I understand correctly) I plan to complete backtrace in the original way first, and rewrite the built-in functions such as Abort and exit in the next new PR.
There was a problem hiding this comment.
I temporarily changed pure to impure to match element (I saw the original Abort implementation was the same). Since the previous PR was problematic and had been rejected, I submitted a new PR after making modifications. You can follow this PR to follow the latest changes.#118179
There was a problem hiding this comment.
I do not think that Exit can be rewritten without front-end support because it has an OPTIONAL argument, so it requires an explicit interface. Maybe Abort could be handled without front-end support, however, I think its NORETURN aspect is something that may be valuable to lowering (not leveraged yet), so it may be worth keeping it like this. @klausler, do you agree Abort and Exit need or benefit from front-end support, while Backtrace does not?
Hey guys, I found that Flang's built-in ABORT function is incomplete when I was using it. Compared with gfortran's ABORT (which can both abort and print out a backtrace), flang's ABORT implementation lacks the function of printing out a backtrace. This feature is essential for debugging and understanding the call stack at the failure point.
To solve this problem, I completed the "// TODO:" of the abort function, and then implemented an additional built-in function BACKTRACE for flang. After a brief reading of the relevant source code, I used backtrace and backtrace_symbols in "execinfo.h" to quickly implement this. But since I used the above two functions directly, my implementation is slightly different from gfortran's implementation (in the output, the function call stack before main is additionally output, and the function line number is missing). In addition, since I used the above two functions, I did not need to add -g to embed debug information into the ELF file, but needed -rdynamic to ensure that the symbols are added to the dynamic symbol table (so that the function name will be printed out).
Here is a comparison of the output between gfortran 's backtrace and my implementation:
gfortran's implemention output:
my impelmention output:
test program is:
I am well aware of the importance of line numbers, so I am now working on implementing line numbers (by parsing DWARF information) and supporting cross-platform (Windows) support.